Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Use const references to options instead of pointers in QgsVectorLayer…
…Exporter

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.
  • Loading branch information
nyalldawson committed Aug 17, 2017
1 parent 26a911c commit c7affb3
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 30 deletions.
8 changes: 4 additions & 4 deletions python/core/qgsvectorlayerexporter.sip
Expand Up @@ -52,7 +52,7 @@ class QgsVectorLayerExporter : QgsFeatureSink
const QgsCoordinateReferenceSystem &destCRS,
bool onlySelected = false,
QString *errorMessage /Out/ = 0,
QMap<QString, QVariant> *options = 0,
const QMap<QString, QVariant> &options = QMap<QString, QVariant>(),
QgsFeedback *feedback = 0
);
%Docstring
Expand All @@ -76,7 +76,7 @@ class QgsVectorLayerExporter : QgsFeatureSink
QgsWkbTypes::Type geometryType,
const QgsCoordinateReferenceSystem &crs,
bool overwrite = false,
const QMap<QString, QVariant> *options = 0 );
const QMap<QString, QVariant> &options = QMap<QString, QVariant>() );
%Docstring
Constructor for QgsVectorLayerExporter.
\param uri URI for destination data source
Expand Down Expand Up @@ -152,7 +152,7 @@ class QgsVectorLayerExporterTask : QgsTask
const QString &uri,
const QString &providerKey,
const QgsCoordinateReferenceSystem &destinationCrs,
QMap<QString, QVariant> *options = 0,
const QMap<QString, QVariant> &options = QMap<QString, QVariant>(),
bool ownsLayer = false );
%Docstring
Constructor for QgsVectorLayerExporterTask. Takes a source ``layer``, destination ``uri``
Expand All @@ -165,7 +165,7 @@ class QgsVectorLayerExporterTask : QgsTask
const QString &uri,
const QString &providerKey,
const QgsCoordinateReferenceSystem &destinationCrs,
QMap<QString, QVariant> *options = 0 ) /Factory/;
const QMap<QString, QVariant> &options = QMap<QString, QVariant>() ) /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
Expand Down
2 changes: 1 addition & 1 deletion src/core/processing/qgsprocessingutils.cpp
Expand Up @@ -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;

Expand Down
27 changes: 14 additions & 13 deletions src/core/qgsvectorlayerexporter.cpp
Expand Up @@ -51,7 +51,7 @@ QgsVectorLayerExporter::QgsVectorLayerExporter( const QString &uri,
QgsWkbTypes::Type geometryType,
const QgsCoordinateReferenceSystem &crs,
bool overwrite,
const QMap<QString, QVariant> *options )
const QMap<QString, QVariant> &options )
: mErrorCount( 0 )
, mAttributeCount( -1 )

Expand All @@ -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;
Expand All @@ -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=";
Expand Down Expand Up @@ -231,7 +231,7 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer,
const QgsCoordinateReferenceSystem &destCRS,
bool onlySelected,
QString *errorMessage,
QMap<QString, QVariant> *options,
const QMap<QString, QVariant> &options,
QgsFeedback *feedback )
{
QgsCoordinateReferenceSystem outputCRS;
Expand All @@ -256,10 +256,11 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer,

bool overwrite = false;
bool forceSinglePartGeom = false;
if ( options )
QMap<QString, QVariant> 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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -456,21 +457,21 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer,
// QgsVectorLayerExporterTask
//

QgsVectorLayerExporterTask::QgsVectorLayerExporterTask( QgsVectorLayer *layer, const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, QMap<QString, QVariant> *options, bool ownsLayer )
QgsVectorLayerExporterTask::QgsVectorLayerExporterTask( QgsVectorLayer *layer, const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, const QMap<QString, QVariant> &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<QString, QVariant>() )
, 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<QString, QVariant> *options )
QgsVectorLayerExporterTask *QgsVectorLayerExporterTask::withLayerOwnership( QgsVectorLayer *layer, const QString &uri, const QString &providerKey, const QgsCoordinateReferenceSystem &destinationCrs, const QMap<QString, QVariant> &options )
{
std::unique_ptr< QgsVectorLayerExporterTask > newTask( new QgsVectorLayerExporterTask( layer, uri, providerKey, destinationCrs, options ) );
newTask->mOwnsLayer = true;
Expand All @@ -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;
}
Expand Down
8 changes: 4 additions & 4 deletions src/core/qgsvectorlayerexporter.h
Expand Up @@ -84,7 +84,7 @@ class CORE_EXPORT QgsVectorLayerExporter : public QgsFeatureSink
const QgsCoordinateReferenceSystem &destCRS,
bool onlySelected = false,
QString *errorMessage SIP_OUT = 0,
QMap<QString, QVariant> *options = nullptr,
const QMap<QString, QVariant> &options = QMap<QString, QVariant>(),
QgsFeedback *feedback = nullptr
);

Expand All @@ -105,7 +105,7 @@ class CORE_EXPORT QgsVectorLayerExporter : public QgsFeatureSink
QgsWkbTypes::Type geometryType,
const QgsCoordinateReferenceSystem &crs,
bool overwrite = false,
const QMap<QString, QVariant> *options = nullptr );
const QMap<QString, QVariant> &options = QMap<QString, QVariant>() );

//! QgsVectorLayerExporter cannot be copied
QgsVectorLayerExporter( const QgsVectorLayerExporter &rh ) = delete;
Expand Down Expand Up @@ -195,7 +195,7 @@ class CORE_EXPORT QgsVectorLayerExporterTask : public QgsTask
const QString &uri,
const QString &providerKey,
const QgsCoordinateReferenceSystem &destinationCrs,
QMap<QString, QVariant> *options = nullptr,
const QMap<QString, QVariant> &options = QMap<QString, QVariant>(),
bool ownsLayer = false );

/**
Expand All @@ -208,7 +208,7 @@ class CORE_EXPORT QgsVectorLayerExporterTask : public QgsTask
const QString &uri,
const QString &providerKey,
const QgsCoordinateReferenceSystem &destinationCrs,
QMap<QString, QVariant> *options = nullptr ) SIP_FACTORY;
const QMap<QString, QVariant> &options = QMap<QString, QVariant>() ) SIP_FACTORY;

virtual void cancel() override;

Expand Down
12 changes: 6 additions & 6 deletions src/providers/ogr/qgsgeopackagedataitems.cpp
Expand Up @@ -322,13 +322,13 @@ bool QgsGeoPackageConnectionItem::handleDrop( const QMimeData *data, Qt::DropAct
tr( "Destination layer <b>%1</b> already exists. Do you want to overwrite it?" ).arg( u.name ), QMessageBox::Yes | QMessageBox::No ) == QMessageBox::Yes )
{

std::unique_ptr< QMap<QString, QVariant> > options( new QMap<QString, QVariant> );
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, [ = ]()
Expand Down
2 changes: 1 addition & 1 deletion src/providers/postgres/qgspostgresdataitems.cpp
Expand Up @@ -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, [ = ]()
Expand Down
2 changes: 1 addition & 1 deletion src/providers/spatialite/qgsspatialitedataitems.cpp
Expand Up @@ -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, [ = ]()
Expand Down

0 comments on commit c7affb3

Please sign in to comment.