Modernize paste to memory layer code

nyalldawson · nyalldawson · commit dbe45b8d5e70 · 2018-02-05T12:19:13.000+11:00
diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -8220,18 +8220,15 @@ void QgisApp::pasteFromClipboard( QgsMapLayer *destinationLayer )
 void QgisApp::pasteAsNewVector()
 {
 
-  QgsVectorLayer *layer = pasteToNewMemoryVector();
+  std::unique_ptr< QgsVectorLayer > layer = pasteToNewMemoryVector();
   if ( !layer )
     return;
 
-  saveAsVectorFileGeneral( layer, false );
-
-  delete layer;
+  saveAsVectorFileGeneral( layer.get(), false );
 }
 
 QgsVectorLayer *QgisApp::pasteAsNewMemoryVector( const QString &layerName )
 {
-
   QString layerNameCopy = layerName;
 
   if ( layerNameCopy.isEmpty() )
@@ -8250,36 +8247,36 @@ QgsVectorLayer *QgisApp::pasteAsNewMemoryVector( const QString &layerName )
     }
   }
 
-  QgsVectorLayer *layer = pasteToNewMemoryVector();
+  std::unique_ptr< QgsVectorLayer > layer = pasteToNewMemoryVector();
   if ( !layer )
     return nullptr;
 
   layer->setName( layerNameCopy );
 
   freezeCanvases();
 
-  QgsProject::instance()->addMapLayer( layer );
+  QgsVectorLayer *result = layer.get();
+  QgsProject::instance()->addMapLayer( layer.release() );
 
   freezeCanvases( false );
   refreshMapCanvas();
 
-  return layer;
+  return result;
 }
 
-QgsVectorLayer *QgisApp::pasteToNewMemoryVector()
+std::unique_ptr<QgsVectorLayer> QgisApp::pasteToNewMemoryVector()
 {
-  QgsFields fields = clipboard()->fields();
+  const QgsFields fields = clipboard()->fields();
 
   // Decide geometry type from features, switch to multi type if at least one multi is found
   QMap<QgsWkbTypes::Type, int> typeCounts;
-  QgsFeatureList features = clipboard()->copyOf( fields );
-  for ( int i = 0; i < features.size(); i++ )
+  const QgsFeatureList features = clipboard()->copyOf( fields );
+  for ( const QgsFeature &feature : features )
   {
-    QgsFeature &feature = features[i];
     if ( !feature.hasGeometry() )
       continue;
 
-    QgsWkbTypes::Type type = feature.geometry().wkbType();
+    const QgsWkbTypes::Type type = feature.geometry().wkbType();
 
     if ( type == QgsWkbTypes::Unknown || type == QgsWkbTypes::NoGeometry )
       continue;
@@ -8307,7 +8304,7 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()
     }
   }
 
-  QgsWkbTypes::Type wkbType = !typeCounts.isEmpty() ? typeCounts.keys().value( 0 ) : QgsWkbTypes::NoGeometry;
+  const QgsWkbTypes::Type wkbType = !typeCounts.isEmpty() ? typeCounts.keys().value( 0 ) : QgsWkbTypes::NoGeometry;
 
   if ( features.isEmpty() )
   {
@@ -8325,39 +8322,38 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()
                                QgsMessageBar::INFO, messageTimeout() );
   }
 
-  QgsVectorLayer *layer = QgsMemoryProviderUtils::createMemoryLayer( QStringLiteral( "pasted_features" ), QgsFields(), wkbType, clipboard()->crs() );
+  std::unique_ptr< QgsVectorLayer > layer( QgsMemoryProviderUtils::createMemoryLayer( QStringLiteral( "pasted_features" ), QgsFields(), wkbType, clipboard()->crs() ) );
 
   if ( !layer->isValid() || !layer->dataProvider() )
   {
-    delete layer;
     messageBar()->pushMessage( tr( "Paste features" ),
                                tr( "Cannot create new layer." ),
                                QgsMessageBar::WARNING, messageTimeout() );
     return nullptr;
   }
 
   layer->startEditing();
-  Q_FOREACH ( QgsField f, clipboard()->fields().toList() )
+  for ( const QgsField &f : clipboard()->fields() )
   {
     QgsDebugMsg( QString( "field %1 (%2)" ).arg( f.name(), QVariant::typeToName( f.type() ) ) );
     if ( !layer->addAttribute( f ) )
     {
       messageBar()->pushMessage( tr( "Paste features" ),
                                  tr( "Cannot create field %1 (%2,%3)" ).arg( f.name(), f.typeName(), QVariant::typeToName( f.type() ) ),
                                  QgsMessageBar::WARNING, messageTimeout() );
-      delete layer;
       return nullptr;
     }
   }
 
   // Convert to multi if necessary
-  for ( int i = 0; i < features.size(); i++ )
+  QgsFeatureList convertedFeatures;
+  convertedFeatures.reserve( features.length() );
+  for ( QgsFeature feature : features )
   {
-    QgsFeature &feature = features[i];
     if ( !feature.hasGeometry() )
       continue;
 
-    QgsWkbTypes::Type type = feature.geometry().wkbType();
+    const QgsWkbTypes::Type type = feature.geometry().wkbType();
     if ( type == QgsWkbTypes::Unknown || type == QgsWkbTypes::NoGeometry )
       continue;
 
@@ -8372,11 +8368,11 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()
       g.convertToMultiType();
       feature.setGeometry( g );
     }
+    convertedFeatures.append( feature );
   }
-  if ( ! layer->addFeatures( features ) || !layer->commitChanges() )
+  if ( ! layer->addFeatures( convertedFeatures ) || !layer->commitChanges() )
   {
     QgsDebugMsg( "Cannot add features or commit changes" );
-    delete layer;
     return nullptr;
   }
 
diff --git a/src/app/qgisapp.h b/src/app/qgisapp.h
@@ -1751,10 +1751,10 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
 
     /**
      * Paste features from clipboard into a new memory layer.
-     *  If no features are in clipboard an empty layer is returned.
-     *  \returns pointer to a new layer or 0 if failed
+     * If no features are in clipboard an empty layer is returned.
+     * Returns a new memory layer or a nullptr if the operation failed.
      */
-    QgsVectorLayer *pasteToNewMemoryVector();
+    std::unique_ptr< QgsVectorLayer > pasteToNewMemoryVector();
 
     //! Returns all annotation items in the canvas
     QList<QgsMapCanvasAnnotationItem *> annotationItems();

Original file line number	Diff line number	Diff line change
`@@ -8220,18 +8220,15 @@ void QgisApp::pasteFromClipboard( QgsMapLayer *destinationLayer )`
`8220`	`8220`	`void QgisApp::pasteAsNewVector()`
`8221`	`8221`	`{`
`8222`	`8222`
`8223`		`- QgsVectorLayer *layer = pasteToNewMemoryVector();`
	`8223`	`+ std::unique_ptr< QgsVectorLayer > layer = pasteToNewMemoryVector();`
`8224`	`8224`	`if ( !layer )`
`8225`	`8225`	`return;`
`8226`	`8226`
`8227`		`- saveAsVectorFileGeneral( layer, false );`
`8228`		`-`
`8229`		`- delete layer;`
	`8227`	`+ saveAsVectorFileGeneral( layer.get(), false );`
`8230`	`8228`	`}`
`8231`	`8229`
`8232`	`8230`	`QgsVectorLayer *QgisApp::pasteAsNewMemoryVector( const QString &layerName )`
`8233`	`8231`	`{`
`8234`		`-`
`8235`	`8232`	`QString layerNameCopy = layerName;`
`8236`	`8233`
`8237`	`8234`	`if ( layerNameCopy.isEmpty() )`
`@@ -8250,36 +8247,36 @@ QgsVectorLayer *QgisApp::pasteAsNewMemoryVector( const QString &layerName )`
`8250`	`8247`	`}`
`8251`	`8248`	`}`
`8252`	`8249`
`8253`		`- QgsVectorLayer *layer = pasteToNewMemoryVector();`
	`8250`	`+ std::unique_ptr< QgsVectorLayer > layer = pasteToNewMemoryVector();`
`8254`	`8251`	`if ( !layer )`
`8255`	`8252`	`return nullptr;`
`8256`	`8253`
`8257`	`8254`	`layer->setName( layerNameCopy );`
`8258`	`8255`
`8259`	`8256`	`freezeCanvases();`
`8260`	`8257`
`8261`		`- QgsProject::instance()->addMapLayer( layer );`
	`8258`	`+ QgsVectorLayer *result = layer.get();`
	`8259`	`+ QgsProject::instance()->addMapLayer( layer.release() );`
`8262`	`8260`
`8263`	`8261`	`freezeCanvases( false );`
`8264`	`8262`	`refreshMapCanvas();`
`8265`	`8263`
`8266`		`- return layer;`
	`8264`	`+ return result;`
`8267`	`8265`	`}`
`8268`	`8266`
`8269`		`-QgsVectorLayer *QgisApp::pasteToNewMemoryVector()`
	`8267`	`+std::unique_ptr<QgsVectorLayer> QgisApp::pasteToNewMemoryVector()`
`8270`	`8268`	`{`
`8271`		`- QgsFields fields = clipboard()->fields();`
	`8269`	`+ const QgsFields fields = clipboard()->fields();`
`8272`	`8270`
`8273`	`8271`	`// Decide geometry type from features, switch to multi type if at least one multi is found`
`8274`	`8272`	`QMap<QgsWkbTypes::Type, int> typeCounts;`
`8275`		`- QgsFeatureList features = clipboard()->copyOf( fields );`
`8276`		`- for ( int i = 0; i < features.size(); i++ )`
	`8273`	`+ const QgsFeatureList features = clipboard()->copyOf( fields );`
	`8274`	`+ for ( const QgsFeature &feature : features )`
`8277`	`8275`	`{`
`8278`		`- QgsFeature &feature = features[i];`
`8279`	`8276`	`if ( !feature.hasGeometry() )`
`8280`	`8277`	`continue;`
`8281`	`8278`
`8282`		`- QgsWkbTypes::Type type = feature.geometry().wkbType();`
	`8279`	`+ const QgsWkbTypes::Type type = feature.geometry().wkbType();`
`8283`	`8280`
`8284`	`8281`	`if ( type == QgsWkbTypes::Unknown \|\| type == QgsWkbTypes::NoGeometry )`
`8285`	`8282`	`continue;`
`@@ -8307,7 +8304,7 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()`
`8307`	`8304`	`}`
`8308`	`8305`	`}`
`8309`	`8306`
`8310`		`- QgsWkbTypes::Type wkbType = !typeCounts.isEmpty() ? typeCounts.keys().value( 0 ) : QgsWkbTypes::NoGeometry;`
	`8307`	`+ const QgsWkbTypes::Type wkbType = !typeCounts.isEmpty() ? typeCounts.keys().value( 0 ) : QgsWkbTypes::NoGeometry;`
`8311`	`8308`
`8312`	`8309`	`if ( features.isEmpty() )`
`8313`	`8310`	`{`
`@@ -8325,39 +8322,38 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()`
`8325`	`8322`	`QgsMessageBar::INFO, messageTimeout() );`
`8326`	`8323`	`}`
`8327`	`8324`
`8328`		`- QgsVectorLayer *layer = QgsMemoryProviderUtils::createMemoryLayer( QStringLiteral( "pasted_features" ), QgsFields(), wkbType, clipboard()->crs() );`
	`8325`	`+ std::unique_ptr< QgsVectorLayer > layer( QgsMemoryProviderUtils::createMemoryLayer( QStringLiteral( "pasted_features" ), QgsFields(), wkbType, clipboard()->crs() ) );`
`8329`	`8326`
`8330`	`8327`	`if ( !layer->isValid() \|\| !layer->dataProvider() )`
`8331`	`8328`	`{`
`8332`		`- delete layer;`
`8333`	`8329`	`messageBar()->pushMessage( tr( "Paste features" ),`
`8334`	`8330`	`tr( "Cannot create new layer." ),`
`8335`	`8331`	`QgsMessageBar::WARNING, messageTimeout() );`
`8336`	`8332`	`return nullptr;`
`8337`	`8333`	`}`
`8338`	`8334`
`8339`	`8335`	`layer->startEditing();`
`8340`		`- Q_FOREACH ( QgsField f, clipboard()->fields().toList() )`
	`8336`	`+ for ( const QgsField &f : clipboard()->fields() )`
`8341`	`8337`	`{`
`8342`	`8338`	`QgsDebugMsg( QString( "field %1 (%2)" ).arg( f.name(), QVariant::typeToName( f.type() ) ) );`
`8343`	`8339`	`if ( !layer->addAttribute( f ) )`
`8344`	`8340`	`{`
`8345`	`8341`	`messageBar()->pushMessage( tr( "Paste features" ),`
`8346`	`8342`	`tr( "Cannot create field %1 (%2,%3)" ).arg( f.name(), f.typeName(), QVariant::typeToName( f.type() ) ),`
`8347`	`8343`	`QgsMessageBar::WARNING, messageTimeout() );`
`8348`		`- delete layer;`
`8349`	`8344`	`return nullptr;`
`8350`	`8345`	`}`
`8351`	`8346`	`}`
`8352`	`8347`
`8353`	`8348`	`// Convert to multi if necessary`
`8354`		`- for ( int i = 0; i < features.size(); i++ )`
	`8349`	`+ QgsFeatureList convertedFeatures;`
	`8350`	`+ convertedFeatures.reserve( features.length() );`
	`8351`	`+ for ( QgsFeature feature : features )`
`8355`	`8352`	`{`
`8356`		`- QgsFeature &feature = features[i];`
`8357`	`8353`	`if ( !feature.hasGeometry() )`
`8358`	`8354`	`continue;`
`8359`	`8355`
`8360`		`- QgsWkbTypes::Type type = feature.geometry().wkbType();`
	`8356`	`+ const QgsWkbTypes::Type type = feature.geometry().wkbType();`
`8361`	`8357`	`if ( type == QgsWkbTypes::Unknown \|\| type == QgsWkbTypes::NoGeometry )`
`8362`	`8358`	`continue;`
`8363`	`8359`
`@@ -8372,11 +8368,11 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()`
`8372`	`8368`	`g.convertToMultiType();`
`8373`	`8369`	`feature.setGeometry( g );`
`8374`	`8370`	`}`
	`8371`	`+ convertedFeatures.append( feature );`
`8375`	`8372`	`}`
`8376`		`- if ( ! layer->addFeatures( features ) \|\| !layer->commitChanges() )`
	`8373`	`+ if ( ! layer->addFeatures( convertedFeatures ) \|\| !layer->commitChanges() )`
`8377`	`8374`	`{`
`8378`	`8375`	`QgsDebugMsg( "Cannot add features or commit changes" );`
`8379`		`- delete layer;`
`8380`	`8376`	`return nullptr;`
`8381`	`8377`	`}`
`8382`	`8378`