Cleanup: QGeoServiceProvider

Use member initialization and std::unique_ptr instead of explicitly
initializing pointers.

Clean up the creating helper template, it doesn't need a pointer to the
existing manager, but don't call it if the manager is already created.

Change-Id: Ia45587cbd36838f1fcdfbb0407100391f0977300
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
Reviewed-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io>
(cherry picked from commit 49a72acedbe4f66c94aca39f8ce9b2ea1e694366)
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

2 files changed, 57 insertions, 77 deletions

diff --git a/src/location/maps/qgeoserviceprovider.cpp b/src/location/maps/qgeoserviceprovider.cpp
index ea838deb..3330a3fb 100644
--- a/src/location/maps/qgeoserviceprovider.cpp
+++ b/src/location/maps/qgeoserviceprovider.cpp
@@ -366,19 +366,19 @@ template <> QPlaceManagerEngine *createEngine<QPlaceManagerEngine>(QGeoServicePr
  * mappingManager() etc methods */
 template <class Manager, class Engine>
 Manager *QGeoServiceProviderPrivate::manager(QGeoServiceProvider::Error *_error,
-                                             QString *_errorString, Manager **_manager)
+                                             QString *_errorString)
 {
     // make local references just so this method is easier to read
     QGeoServiceProvider::Error &error = *_error;
     QString &errorString = *_errorString;
-    Manager *&manager = *_manager;
+    Manager *manager = nullptr;
 
-    if (!this->factory) {
-        this->filterParameterMap();
-        this->loadPlugin(this->parameterMap);
+    if (!factory) {
+        filterParameterMap();
+        loadPlugin(this->parameterMap);
     }
 
-    if (!this->factory) {
+    if (!factory) {
         error = this->error;
         errorString = this->errorString;
         return nullptr;
@@ -390,15 +390,14 @@ Manager *QGeoServiceProviderPrivate::manager(QGeoServiceProvider::Error *_error,
 
         if (engine) {
             engine->setManagerName(
-                        this->metaData.value(QStringLiteral("Provider")).toString());
+                        metaData.value(QStringLiteral("Provider")).toString());
             engine->setManagerVersion(
-                        int(this->metaData.value(QStringLiteral("Version")).toDouble()));
+                        int(metaData.value(QStringLiteral("Version")).toDouble()));
             manager = new Manager(engine);
         } else if (error == QGeoServiceProvider::NoError) {
             error = QGeoServiceProvider::NotSupportedError;
-            errorString = QLatin1String("The service provider does not support the ");
-            errorString.append(QLatin1String(Manager::staticMetaObject.className()));
-            errorString.append(QLatin1String(" type."));
+            errorString = QLatin1String("The service provider does not support the %1 type.")
+                                      .arg(QLatin1String(Manager::staticMetaObject.className()));
         }
 
         if (error != QGeoServiceProvider::NoError) {
@@ -408,8 +407,8 @@ Manager *QGeoServiceProviderPrivate::manager(QGeoServiceProvider::Error *_error,
             this->errorString = errorString;
         }
 
-        if (manager && this->localeSet)
-            manager->setLocale(this->locale);
+        if (manager && localeSet)
+            manager->setLocale(locale);
     }
 
     if (manager) {
@@ -443,12 +442,13 @@ Manager *QGeoServiceProviderPrivate::manager(QGeoServiceProvider::Error *_error,
 */
 QGeoCodingManager *QGeoServiceProvider::geocodingManager() const
 {
-    QGeoCodingManager *mgr = d_ptr->manager<QGeoCodingManager, QGeoCodingManagerEngine>(
-               &(d_ptr->geocodeError), &(d_ptr->geocodeErrorString),
-               &(d_ptr->geocodingManager));
-    if (!mgr)
-        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
-    return mgr;
+    if (!d_ptr->geocodingManager) {
+        d_ptr->geocodingManager.reset(d_ptr->manager<QGeoCodingManager, QGeoCodingManagerEngine>(
+               &(d_ptr->geocodeError), &(d_ptr->geocodeErrorString)));
+        if (!d_ptr->geocodingManager)
+            qDebug() << d_ptr->error << ", " << d_ptr->errorString;
+    }
+    return d_ptr->geocodingManager.get();
 }
 
 /*!
@@ -475,12 +475,13 @@ QGeoCodingManager *QGeoServiceProvider::geocodingManager() const
 */
 QGeoMappingManager *QGeoServiceProvider::mappingManager() const
 {
-    QGeoMappingManager *mgr = d_ptr->manager<QGeoMappingManager, QGeoMappingManagerEngine>(
-               &(d_ptr->mappingError), &(d_ptr->mappingErrorString),
-               &(d_ptr->mappingManager));
-    if (!mgr)
-        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
-    return mgr;
+    if (!d_ptr->mappingManager) {
+        d_ptr->mappingManager.reset(d_ptr->manager<QGeoMappingManager, QGeoMappingManagerEngine>(
+                &(d_ptr->mappingError), &(d_ptr->mappingErrorString)));
+        if (!d_ptr->mappingManager)
+            qDebug() << d_ptr->error << ", " << d_ptr->errorString;
+    }
+    return d_ptr->mappingManager.get();
 }
 
 /*!
@@ -505,12 +506,13 @@ QGeoMappingManager *QGeoServiceProvider::mappingManager() const
 */
 QGeoRoutingManager *QGeoServiceProvider::routingManager() const
 {
-    QGeoRoutingManager *mgr = d_ptr->manager<QGeoRoutingManager, QGeoRoutingManagerEngine>(
-               &(d_ptr->routingError), &(d_ptr->routingErrorString),
-               &(d_ptr->routingManager));
-    if (!mgr)
-        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
-    return mgr;
+    if (!d_ptr->routingManager) {
+        d_ptr->routingManager.reset(d_ptr->manager<QGeoRoutingManager, QGeoRoutingManagerEngine>(
+                &(d_ptr->routingError), &(d_ptr->routingErrorString)));
+        if (!d_ptr->routingManager)
+            qDebug() << d_ptr->error << ", " << d_ptr->errorString;
+    }
+    return d_ptr->routingManager.get();
 }
 
 /*!
@@ -531,12 +533,13 @@ QGeoRoutingManager *QGeoServiceProvider::routingManager() const
 */
 QPlaceManager *QGeoServiceProvider::placeManager() const
 {
-    QPlaceManager *mgr = d_ptr->manager<QPlaceManager, QPlaceManagerEngine>(
-               &(d_ptr->placeError), &(d_ptr->placeErrorString),
-                &(d_ptr->placeManager));
-    if (!mgr)
-        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
-    return mgr;
+    if (!d_ptr->placeManager) {
+        d_ptr->placeManager.reset(d_ptr->manager<QPlaceManager, QPlaceManagerEngine>(
+                &(d_ptr->placeError), &(d_ptr->placeErrorString)));
+        if (!d_ptr->placeManager)
+            qDebug() << d_ptr->error << ", " << d_ptr->errorString;
+    }
+    return d_ptr->placeManager.get();
 }
 
 /*!
@@ -731,43 +734,20 @@ void QGeoServiceProvider::setLocale(const QLocale &locale)
 *******************************************************************************/
 
 QGeoServiceProviderPrivate::QGeoServiceProviderPrivate()
-    : factory(nullptr),
-      experimental(false),
-      geocodingManager(nullptr),
-      routingManager(nullptr),
-      mappingManager(nullptr),
-      placeManager(nullptr),
-      geocodeError(QGeoServiceProvider::NoError),
-      routingError(QGeoServiceProvider::NoError),
-      mappingError(QGeoServiceProvider::NoError),
-      placeError(QGeoServiceProvider::NoError),
-      error(QGeoServiceProvider::NoError),
-      localeSet(false)
 {
     metaData.insert(QStringLiteral("index"), -1);
 }
 
 QGeoServiceProviderPrivate::~QGeoServiceProviderPrivate()
 {
-    delete geocodingManager;
-    delete routingManager;
-    delete mappingManager;
-    delete placeManager;
 }
 
 void QGeoServiceProviderPrivate::unload()
 {
-    delete geocodingManager;
-    geocodingManager = nullptr;
-
-    delete routingManager;
-    routingManager = nullptr;
-
-    delete mappingManager;
-    mappingManager = nullptr;
-
-    delete placeManager;
-    placeManager = nullptr;
+    geocodingManager.reset();
+    routingManager.reset();
+    mappingManager.reset();
+    placeManager.reset();
 
     factory = nullptr;
     error = QGeoServiceProvider::NoError;
diff --git a/src/location/maps/qgeoserviceprovider_p.h b/src/location/maps/qgeoserviceprovider_p.h
index 03c0a798..a563b183 100644
--- a/src/location/maps/qgeoserviceprovider_p.h
+++ b/src/location/maps/qgeoserviceprovider_p.h
@@ -82,28 +82,28 @@ public:
     /* helper templates for generating the feature and manager accessors */
     template <class Manager, class Engine>
     Manager *manager(QGeoServiceProvider::Error *error,
-                     QString *errorString, Manager **manager);
+                     QString *errorString);
     template <class Flags>
     Flags features(const char *enumName) const;
 
-    QGeoServiceProviderFactory *factory;
+    QGeoServiceProviderFactory *factory = nullptr;
     QJsonObject metaData;
 
     QVariantMap parameterMap;
     QVariantMap cleanedParameterMap;
 
-    bool experimental;
+    bool experimental = false;
 
-    QGeoCodingManager *geocodingManager;
-    QGeoRoutingManager *routingManager;
-    QGeoMappingManager *mappingManager;
-    QPlaceManager *placeManager;
+    std::unique_ptr<QGeoCodingManager> geocodingManager;
+    std::unique_ptr<QGeoRoutingManager> routingManager;
+    std::unique_ptr<QGeoMappingManager> mappingManager;
+    std::unique_ptr<QPlaceManager> placeManager;
     QQmlEngine *qmlEngine = nullptr;
 
-    QGeoServiceProvider::Error geocodeError;
-    QGeoServiceProvider::Error routingError;
-    QGeoServiceProvider::Error mappingError;
-    QGeoServiceProvider::Error placeError;
+    QGeoServiceProvider::Error geocodeError = QGeoServiceProvider::NoError;
+    QGeoServiceProvider::Error routingError = QGeoServiceProvider::NoError;
+    QGeoServiceProvider::Error mappingError = QGeoServiceProvider::NoError;
+    QGeoServiceProvider::Error placeError = QGeoServiceProvider::NoError;
     QGeoServiceProvider::Error navigationError = QGeoServiceProvider::NoError;
 
     QString geocodeErrorString;
@@ -112,13 +112,13 @@ public:
     QString placeErrorString;
     QString navigationErrorString;
 
-    QGeoServiceProvider::Error error;
+    QGeoServiceProvider::Error error = QGeoServiceProvider::NoError;
     QString errorString;
 
     QString providerName;
 
     QLocale locale;
-    bool localeSet;
+    bool localeSet = false;
 
     static QMultiHash<QString, QJsonObject> plugins(bool reload = false);
     static void loadPluginMetadata(QMultiHash<QString, QJsonObject> &list);