Avoid multiple layer access calls where possible

nyalldawson · nyalldawson · commit 1f1e8b5de70a · 2023-10-24T19:06:12.000+10:00
diff --git a/src/analysis/processing/qgsalgorithmmergevector.cpp b/src/analysis/processing/qgsalgorithmmergevector.cpp
@@ -109,39 +109,43 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame
 
     QgsVectorLayer *vl = qobject_cast< QgsVectorLayer * >( layer );
 
-    if ( !outputCrs.isValid() && vl->crs().isValid() )
+    const Qgis::WkbType layerWkbType = vl->wkbType();
+    const QgsCoordinateReferenceSystem layerCrs = vl->crs();
+    const QString layerName = vl->name();
+
+    if ( !outputCrs.isValid() && layerCrs.isValid() )
     {
-      outputCrs = vl->crs();
+      outputCrs = layerCrs;
       feedback->pushInfo( QObject::tr( "Taking destination CRS %1 from layer" ).arg( outputCrs.authid() ) );
     }
 
     // check wkb type
     if ( outputType != Qgis::WkbType::Unknown && outputType != Qgis::WkbType::NoGeometry )
     {
-      if ( QgsWkbTypes::geometryType( outputType ) != QgsWkbTypes::geometryType( vl->wkbType() ) )
+      if ( QgsWkbTypes::geometryType( outputType ) != QgsWkbTypes::geometryType( layerWkbType ) )
         throw QgsProcessingException( QObject::tr( "All layers must have same geometry type! Encountered a %1 layer when expecting a %2 layer." )
-                                      .arg( QgsWkbTypes::geometryDisplayString( QgsWkbTypes::geometryType( vl->wkbType() ) ),
+                                      .arg( QgsWkbTypes::geometryDisplayString( QgsWkbTypes::geometryType( layerWkbType ) ),
                                             QgsWkbTypes::geometryDisplayString( QgsWkbTypes::geometryType( outputType ) ) ) );
 
-      if ( QgsWkbTypes::hasM( vl->wkbType() ) && !QgsWkbTypes::hasM( outputType ) )
+      if ( QgsWkbTypes::hasM( layerWkbType ) && !QgsWkbTypes::hasM( outputType ) )
       {
         outputType = QgsWkbTypes::addM( outputType );
         feedback->pushInfo( QObject::tr( "Found a layer with M values, upgrading output type to %1" ).arg( QgsWkbTypes::displayString( outputType ) ) );
       }
-      if ( QgsWkbTypes::hasZ( vl->wkbType() ) && !QgsWkbTypes::hasZ( outputType ) )
+      if ( QgsWkbTypes::hasZ( layerWkbType ) && !QgsWkbTypes::hasZ( outputType ) )
       {
         outputType = QgsWkbTypes::addZ( outputType );
         feedback->pushInfo( QObject::tr( "Found a layer with Z values, upgrading output type to %1" ).arg( QgsWkbTypes::displayString( outputType ) ) );
       }
-      if ( QgsWkbTypes::isMultiType( vl->wkbType() ) && !QgsWkbTypes::isMultiType( outputType ) )
+      if ( QgsWkbTypes::isMultiType( layerWkbType ) && !QgsWkbTypes::isMultiType( outputType ) )
       {
         outputType = QgsWkbTypes::multiType( outputType );
         feedback->pushInfo( QObject::tr( "Found a layer with multiparts, upgrading output type to %1" ).arg( QgsWkbTypes::displayString( outputType ) ) );
       }
     }
     else
     {
-      outputType = vl->wkbType();
+      outputType = layerWkbType;
       feedback->pushInfo( QObject::tr( "Setting output type to %1" ).arg( QgsWkbTypes::displayString( outputType ) ) );
     }
 
@@ -160,7 +164,7 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame
           {
             feedback->pushWarning( QObject::tr( "%1 field in layer %2 has different data type than the destination layer (%3 instead of %4). "
                                                 "%1 field will be converted to string type." )
-                                   .arg( sourceField.name(), vl->name(), sourceField.typeName(), destField.typeName() ) );
+                                   .arg( sourceField.name(), layerName, sourceField.typeName(), destField.typeName() ) );
             destField.setType( QVariant::String );
             destField.setSubType( QVariant::Invalid );
             destField.setLength( 0 );
@@ -209,7 +213,11 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame
     if ( !vl )
       continue;
 
-    feedback->pushInfo( QObject::tr( "Packaging layer %1/%2: %3" ).arg( layerNumber ).arg( layers.count() ).arg( layer->name() ) );
+    const QString layerName = layer->name();
+    const QString layerSource = layer->publicSource();
+    const QgsFields layerFields = vl->fields();
+
+    feedback->pushInfo( QObject::tr( "Packaging layer %1/%2: %3" ).arg( layerNumber ).arg( layers.count() ).arg( layerName ) );
 
     QgsFeatureIterator it = vl->getFeatures( QgsFeatureRequest().setDestinationCrs( outputCrs, context.transformContext() ) );
     QgsFeature f;
@@ -248,17 +256,17 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame
       {
         if ( addLayerField && destField.name() == QLatin1String( "layer" ) )
         {
-          destAttributes.append( layer->name() );
+          destAttributes.append( layerName );
           continue;
         }
         else if ( addPathField && destField.name() == QLatin1String( "path" ) )
         {
-          destAttributes.append( layer->publicSource() );
+          destAttributes.append( layerSource );
           continue;
         }
 
         QVariant destAttribute;
-        const int sourceIndex = vl->fields().lookupField( destField.name() );
+        const int sourceIndex = layerFields.lookupField( destField.name() );
         if ( sourceIndex >= 0 )
         {
           destAttribute = f.attributes().at( sourceIndex );

Original file line number	Diff line number	Diff line change
`@@ -109,39 +109,43 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame`
`109`	`109`
`110`	`110`	`QgsVectorLayer vl = qobject_cast< QgsVectorLayer >( layer );`
`111`	`111`
`112`		`- if ( !outputCrs.isValid() && vl->crs().isValid() )`
	`112`	`+ const Qgis::WkbType layerWkbType = vl->wkbType();`
	`113`	`+ const QgsCoordinateReferenceSystem layerCrs = vl->crs();`
	`114`	`+ const QString layerName = vl->name();`
	`115`	`+`
	`116`	`+ if ( !outputCrs.isValid() && layerCrs.isValid() )`
`113`	`117`	`{`
`114`		`- outputCrs = vl->crs();`
	`118`	`+ outputCrs = layerCrs;`
`115`	`119`	`feedback->pushInfo( QObject::tr( "Taking destination CRS %1 from layer" ).arg( outputCrs.authid() ) );`
`116`	`120`	`}`
`117`	`121`
`118`	`122`	`// check wkb type`
`119`	`123`	`if ( outputType != Qgis::WkbType::Unknown && outputType != Qgis::WkbType::NoGeometry )`
`120`	`124`	`{`
`121`		`- if ( QgsWkbTypes::geometryType( outputType ) != QgsWkbTypes::geometryType( vl->wkbType() ) )`
	`125`	`+ if ( QgsWkbTypes::geometryType( outputType ) != QgsWkbTypes::geometryType( layerWkbType ) )`
`122`	`126`	`throw QgsProcessingException( QObject::tr( "All layers must have same geometry type! Encountered a %1 layer when expecting a %2 layer." )`
`123`		`- .arg( QgsWkbTypes::geometryDisplayString( QgsWkbTypes::geometryType( vl->wkbType() ) ),`
	`127`	`+ .arg( QgsWkbTypes::geometryDisplayString( QgsWkbTypes::geometryType( layerWkbType ) ),`
`124`	`128`	`QgsWkbTypes::geometryDisplayString( QgsWkbTypes::geometryType( outputType ) ) ) );`
`125`	`129`
`126`		`- if ( QgsWkbTypes::hasM( vl->wkbType() ) && !QgsWkbTypes::hasM( outputType ) )`
	`130`	`+ if ( QgsWkbTypes::hasM( layerWkbType ) && !QgsWkbTypes::hasM( outputType ) )`
`127`	`131`	`{`
`128`	`132`	`outputType = QgsWkbTypes::addM( outputType );`
`129`	`133`	`feedback->pushInfo( QObject::tr( "Found a layer with M values, upgrading output type to %1" ).arg( QgsWkbTypes::displayString( outputType ) ) );`
`130`	`134`	`}`
`131`		`- if ( QgsWkbTypes::hasZ( vl->wkbType() ) && !QgsWkbTypes::hasZ( outputType ) )`
	`135`	`+ if ( QgsWkbTypes::hasZ( layerWkbType ) && !QgsWkbTypes::hasZ( outputType ) )`
`132`	`136`	`{`
`133`	`137`	`outputType = QgsWkbTypes::addZ( outputType );`
`134`	`138`	`feedback->pushInfo( QObject::tr( "Found a layer with Z values, upgrading output type to %1" ).arg( QgsWkbTypes::displayString( outputType ) ) );`
`135`	`139`	`}`
`136`		`- if ( QgsWkbTypes::isMultiType( vl->wkbType() ) && !QgsWkbTypes::isMultiType( outputType ) )`
	`140`	`+ if ( QgsWkbTypes::isMultiType( layerWkbType ) && !QgsWkbTypes::isMultiType( outputType ) )`
`137`	`141`	`{`
`138`	`142`	`outputType = QgsWkbTypes::multiType( outputType );`
`139`	`143`	`feedback->pushInfo( QObject::tr( "Found a layer with multiparts, upgrading output type to %1" ).arg( QgsWkbTypes::displayString( outputType ) ) );`
`140`	`144`	`}`
`141`	`145`	`}`
`142`	`146`	`else`
`143`	`147`	`{`
`144`		`- outputType = vl->wkbType();`
	`148`	`+ outputType = layerWkbType;`
`145`	`149`	`feedback->pushInfo( QObject::tr( "Setting output type to %1" ).arg( QgsWkbTypes::displayString( outputType ) ) );`
`146`	`150`	`}`
`147`	`151`
`@@ -160,7 +164,7 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame`
`160`	`164`	`{`
`161`	`165`	`feedback->pushWarning( QObject::tr( "%1 field in layer %2 has different data type than the destination layer (%3 instead of %4). "`
`162`	`166`	`"%1 field will be converted to string type." )`
`163`		`- .arg( sourceField.name(), vl->name(), sourceField.typeName(), destField.typeName() ) );`
	`167`	`+ .arg( sourceField.name(), layerName, sourceField.typeName(), destField.typeName() ) );`
`164`	`168`	`destField.setType( QVariant::String );`
`165`	`169`	`destField.setSubType( QVariant::Invalid );`
`166`	`170`	`destField.setLength( 0 );`
`@@ -209,7 +213,11 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame`
`209`	`213`	`if ( !vl )`
`210`	`214`	`continue;`
`211`	`215`
`212`		`- feedback->pushInfo( QObject::tr( "Packaging layer %1/%2: %3" ).arg( layerNumber ).arg( layers.count() ).arg( layer->name() ) );`
	`216`	`+ const QString layerName = layer->name();`
	`217`	`+ const QString layerSource = layer->publicSource();`
	`218`	`+ const QgsFields layerFields = vl->fields();`
	`219`	`+`
	`220`	`+ feedback->pushInfo( QObject::tr( "Packaging layer %1/%2: %3" ).arg( layerNumber ).arg( layers.count() ).arg( layerName ) );`
`213`	`221`
`214`	`222`	`QgsFeatureIterator it = vl->getFeatures( QgsFeatureRequest().setDestinationCrs( outputCrs, context.transformContext() ) );`
`215`	`223`	`QgsFeature f;`
`@@ -248,17 +256,17 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame`
`248`	`256`	`{`
`249`	`257`	`if ( addLayerField && destField.name() == QLatin1String( "layer" ) )`
`250`	`258`	`{`
`251`		`- destAttributes.append( layer->name() );`
	`259`	`+ destAttributes.append( layerName );`
`252`	`260`	`continue;`
`253`	`261`	`}`
`254`	`262`	`else if ( addPathField && destField.name() == QLatin1String( "path" ) )`
`255`	`263`	`{`
`256`		`- destAttributes.append( layer->publicSource() );`
	`264`	`+ destAttributes.append( layerSource );`
`257`	`265`	`continue;`
`258`	`266`	`}`
`259`	`267`
`260`	`268`	`QVariant destAttribute;`
`261`		`- const int sourceIndex = vl->fields().lookupField( destField.name() );`
	`269`	`+ const int sourceIndex = layerFields.lookupField( destField.name() );`
`262`	`270`	`if ( sourceIndex >= 0 )`
`263`	`271`	`{`
`264`	`272`	`destAttribute = f.attributes().at( sourceIndex );`