From c7affb3b70b92b90cbb26169e3aad0869cac9e20 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Thu, 17 Aug 2017 19:16:16 +1000 Subject: [PATCH] Use const references to options instead of pointers in QgsVectorLayerExporter The use of pointers make ownership of the argument confusing, and there's nothing stopping us just using an empty map as the default value instead. --- python/core/qgsvectorlayerexporter.sip | 8 +++--- src/core/processing/qgsprocessingutils.cpp | 2 +- src/core/qgsvectorlayerexporter.cpp | 27 ++++++++++--------- src/core/qgsvectorlayerexporter.h | 8 +++--- src/providers/ogr/qgsgeopackagedataitems.cpp | 12 ++++----- .../postgres/qgspostgresdataitems.cpp | 2 +- .../spatialite/qgsspatialitedataitems.cpp | 2 +- 7 files changed, 31 insertions(+), 30 deletions(-) diff --git a/python/core/qgsvectorlayerexporter.sip b/python/core/qgsvectorlayerexporter.sip index 26822d66818c..afe2eb6fc447 100644 --- a/python/core/qgsvectorlayerexporter.sip +++ b/python/core/qgsvectorlayerexporter.sip @@ -52,7 +52,7 @@ class QgsVectorLayerExporter : QgsFeatureSink const QgsCoordinateReferenceSystem &destCRS, bool onlySelected = false, QString *errorMessage /Out/ = 0, - QMap *options = 0, + const QMap &options = QMap(), QgsFeedback *feedback = 0 ); %Docstring @@ -76,7 +76,7 @@ class QgsVectorLayerExporter : QgsFeatureSink QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, bool overwrite = false, - const QMap *options = 0 ); + const QMap &options = QMap() ); %Docstring Constructor for QgsVectorLayerExporter. \param uri URI for destination data source @@ -152,7 +152,7 @@ class QgsVectorLayerExporterTask : QgsTask const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, - QMap *options = 0, + const QMap &options = QMap(), bool ownsLayer = false ); %Docstring Constructor for QgsVectorLayerExporterTask. Takes a source ``layer``, destination ``uri`` @@ -165,7 +165,7 @@ class QgsVectorLayerExporterTask : QgsTask const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, - QMap *options = 0 ) /Factory/; + const QMap &options = QMap() ) /Factory/; %Docstring Creates a new QgsVectorLayerExporterTask which has ownership over a source ``layer``. When the export task has completed (successfully or otherwise) ``layer`` will be diff --git a/src/core/processing/qgsprocessingutils.cpp b/src/core/processing/qgsprocessingutils.cpp index f4ede6c4fd4b..fe4e0742cac1 100644 --- a/src/core/processing/qgsprocessingutils.cpp +++ b/src/core/processing/qgsprocessingutils.cpp @@ -369,7 +369,7 @@ QgsFeatureSink *QgsProcessingUtils::createFeatureSink( QString &destination, Qgs else { //create empty layer - std::unique_ptr< QgsVectorLayerExporter > exporter( new QgsVectorLayerExporter( uri, providerKey, fields, geometryType, crs, false, &options ) ); + std::unique_ptr< QgsVectorLayerExporter > exporter( new QgsVectorLayerExporter( uri, providerKey, fields, geometryType, crs, false, options ) ); if ( exporter->errorCode() ) return nullptr; diff --git a/src/core/qgsvectorlayerexporter.cpp b/src/core/qgsvectorlayerexporter.cpp index 943c8de2ebd8..88be1917a022 100644 --- a/src/core/qgsvectorlayerexporter.cpp +++ b/src/core/qgsvectorlayerexporter.cpp @@ -51,7 +51,7 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, bool overwrite, - const QMap *options ) + const QMap &options ) : mErrorCount( 0 ) , mAttributeCount( -1 ) @@ -78,7 +78,7 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri, // create an empty layer QString errMsg; - mError = pCreateEmpty( uri, fields, geometryType, crs, overwrite, &mOldToNewAttrIdx, &errMsg, options ); + mError = pCreateEmpty( uri, fields, geometryType, crs, overwrite, &mOldToNewAttrIdx, &errMsg, !options.isEmpty() ? &options : nullptr ); if ( errorCode() ) { mErrorMessage = errMsg; @@ -100,8 +100,8 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri, if ( providerKey == "ogr" ) { QString layerName; - if ( options && options->contains( QStringLiteral( "layerName" ) ) ) - layerName = options->value( QStringLiteral( "layerName" ) ).toString(); + if ( options.contains( QStringLiteral( "layerName" ) ) ) + layerName = options.value( QStringLiteral( "layerName" ) ).toString(); if ( !layerName.isEmpty() ) { uriUpdated += "|layername="; @@ -231,7 +231,7 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer, const QgsCoordinateReferenceSystem &destCRS, bool onlySelected, QString *errorMessage, - QMap *options, + const QMap &options, QgsFeedback *feedback ) { QgsCoordinateReferenceSystem outputCRS; @@ -256,10 +256,11 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer, bool overwrite = false; bool forceSinglePartGeom = false; - if ( options ) + QMap providerOptions = options; + if ( !options.isEmpty() ) { - overwrite = options->take( QStringLiteral( "overwrite" ) ).toBool(); - forceSinglePartGeom = options->take( QStringLiteral( "forceSinglePartGeometryType" ) ).toBool(); + overwrite = providerOptions.take( QStringLiteral( "overwrite" ) ).toBool(); + forceSinglePartGeom = providerOptions.take( QStringLiteral( "forceSinglePartGeometryType" ) ).toBool(); } QgsFields fields = layer->fields(); @@ -304,7 +305,7 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer, } QgsVectorLayerExporter *writer = - new QgsVectorLayerExporter( uri, providerKey, fields, wkbType, outputCRS, overwrite, options ); + new QgsVectorLayerExporter( uri, providerKey, fields, wkbType, outputCRS, overwrite, providerOptions ); // check whether file creation was successful ExportError err = writer->errorCode(); @@ -456,21 +457,21 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer, // QgsVectorLayerExporterTask // -QgsVectorLayerExporterTask::QgsVectorLayerExporterTask( QgsVectorLayer *layer, const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, QMap *options, bool ownsLayer ) +QgsVectorLayerExporterTask::QgsVectorLayerExporterTask( QgsVectorLayer *layer, const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, const QMap &options, bool ownsLayer ) : QgsTask( tr( "Exporting %1" ).arg( layer->name() ), QgsTask::CanCancel ) , mLayer( layer ) , mOwnsLayer( ownsLayer ) , mDestUri( uri ) , mDestProviderKey( providerKey ) , mDestCrs( destinationCrs ) - , mOptions( options ? * options : QMap() ) + , mOptions( options ) , mOwnedFeedback( new QgsFeedback() ) { if ( mLayer ) setDependentLayers( QList< QgsMapLayer * >() << mLayer ); } -QgsVectorLayerExporterTask *QgsVectorLayerExporterTask::withLayerOwnership( QgsVectorLayer *layer, const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, QMap *options ) +QgsVectorLayerExporterTask *QgsVectorLayerExporterTask::withLayerOwnership( QgsVectorLayer *layer, const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, const QMap &options ) { std::unique_ptr< QgsVectorLayerExporterTask > newTask( new QgsVectorLayerExporterTask( layer, uri, providerKey, destinationCrs, options ) ); newTask->mOwnsLayer = true; @@ -493,7 +494,7 @@ bool QgsVectorLayerExporterTask::run() mError = QgsVectorLayerExporter::exportLayer( mLayer.data(), mDestUri, mDestProviderKey, mDestCrs, false, &mErrorMessage, - &mOptions, mOwnedFeedback.get() ); + mOptions, mOwnedFeedback.get() ); return mError == QgsVectorLayerExporter::NoError; } diff --git a/src/core/qgsvectorlayerexporter.h b/src/core/qgsvectorlayerexporter.h index 9e3e6eb74e05..086811655223 100644 --- a/src/core/qgsvectorlayerexporter.h +++ b/src/core/qgsvectorlayerexporter.h @@ -84,7 +84,7 @@ class CORE_EXPORT QgsVectorLayerExporter : public QgsFeatureSink const QgsCoordinateReferenceSystem &destCRS, bool onlySelected = false, QString *errorMessage SIP_OUT = 0, - QMap *options = nullptr, + const QMap &options = QMap(), QgsFeedback *feedback = nullptr ); @@ -105,7 +105,7 @@ class CORE_EXPORT QgsVectorLayerExporter : public QgsFeatureSink QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, bool overwrite = false, - const QMap *options = nullptr ); + const QMap &options = QMap() ); //! QgsVectorLayerExporter cannot be copied QgsVectorLayerExporter( const QgsVectorLayerExporter &rh ) = delete; @@ -195,7 +195,7 @@ class CORE_EXPORT QgsVectorLayerExporterTask : public QgsTask const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, - QMap *options = nullptr, + const QMap &options = QMap(), bool ownsLayer = false ); /** @@ -208,7 +208,7 @@ class CORE_EXPORT QgsVectorLayerExporterTask : public QgsTask const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, - QMap *options = nullptr ) SIP_FACTORY; + const QMap &options = QMap() ) SIP_FACTORY; virtual void cancel() override; diff --git a/src/providers/ogr/qgsgeopackagedataitems.cpp b/src/providers/ogr/qgsgeopackagedataitems.cpp index 7806067f0d5f..80c3b62b55dd 100644 --- a/src/providers/ogr/qgsgeopackagedataitems.cpp +++ b/src/providers/ogr/qgsgeopackagedataitems.cpp @@ -322,13 +322,13 @@ bool QgsGeoPackageConnectionItem::handleDrop( const QMimeData *data, Qt::DropAct tr( "Destination layer %1 already exists. Do you want to overwrite it?" ).arg( u.name ), QMessageBox::Yes | QMessageBox::No ) == QMessageBox::Yes ) { - std::unique_ptr< QMap > options( new QMap ); - options->insert( QStringLiteral( "driverName" ), QStringLiteral( "GPKG" ) ); - options->insert( QStringLiteral( "update" ), true ); - options->insert( QStringLiteral( "overwrite" ), true ); - options->insert( QStringLiteral( "layerName" ), u.name ); + QVariantMap options; + options.insert( QStringLiteral( "driverName" ), QStringLiteral( "GPKG" ) ); + options.insert( QStringLiteral( "update" ), true ); + options.insert( QStringLiteral( "overwrite" ), true ); + options.insert( QStringLiteral( "layerName" ), u.name ); - std::unique_ptr< QgsVectorLayerExporterTask > exportTask( new QgsVectorLayerExporterTask( srcLayer, uri, QStringLiteral( "ogr" ), srcLayer->crs(), options.get(), owner ) ); + std::unique_ptr< QgsVectorLayerExporterTask > exportTask( new QgsVectorLayerExporterTask( srcLayer, uri, QStringLiteral( "ogr" ), srcLayer->crs(), options, owner ) ); // when export is successful: connect( exportTask.get(), &QgsVectorLayerExporterTask::exportComplete, this, [ = ]() diff --git a/src/providers/postgres/qgspostgresdataitems.cpp b/src/providers/postgres/qgspostgresdataitems.cpp index fb8deb10935d..2dc0063d84af 100644 --- a/src/providers/postgres/qgspostgresdataitems.cpp +++ b/src/providers/postgres/qgspostgresdataitems.cpp @@ -244,7 +244,7 @@ bool QgsPGConnectionItem::handleDrop( const QMimeData *data, const QString &toSc uri.setSchema( toSchema ); } - std::unique_ptr< QgsVectorLayerExporterTask > exportTask( new QgsVectorLayerExporterTask( srcLayer, uri.uri( false ), QStringLiteral( "postgres" ), srcLayer->crs(), nullptr, owner ) ); + std::unique_ptr< QgsVectorLayerExporterTask > exportTask( new QgsVectorLayerExporterTask( srcLayer, uri.uri( false ), QStringLiteral( "postgres" ), srcLayer->crs(), QVariantMap(), owner ) ); // when export is successful: connect( exportTask.get(), &QgsVectorLayerExporterTask::exportComplete, this, [ = ]() diff --git a/src/providers/spatialite/qgsspatialitedataitems.cpp b/src/providers/spatialite/qgsspatialitedataitems.cpp index 932e1116bf49..dd0aa3fb08ca 100644 --- a/src/providers/spatialite/qgsspatialitedataitems.cpp +++ b/src/providers/spatialite/qgsspatialitedataitems.cpp @@ -229,7 +229,7 @@ bool QgsSLConnectionItem::handleDrop( const QMimeData *data, Qt::DropAction ) destUri.setDataSource( QString(), u.name, srcLayer->geometryType() != QgsWkbTypes::NullGeometry ? QStringLiteral( "geom" ) : QString() ); QgsDebugMsg( "URI " + destUri.uri() ); - std::unique_ptr< QgsVectorLayerExporterTask > exportTask( new QgsVectorLayerExporterTask( srcLayer, destUri.uri(), QStringLiteral( "spatialite" ), srcLayer->crs(), nullptr, owner ) ); + std::unique_ptr< QgsVectorLayerExporterTask > exportTask( new QgsVectorLayerExporterTask( srcLayer, destUri.uri(), QStringLiteral( "spatialite" ), srcLayer->crs(), QVariantMap(), owner ) ); // when export is successful: connect( exportTask.get(), &QgsVectorLayerExporterTask::exportComplete, this, [ = ]()