[Geometry checker] Fix LayerFeature iterator

qgis · Oct 23, 2017 · f0f66d7 · f0f66d7
1 parent e88f32a
commit f0f66d7
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 59 deletions.
diff --git a/src/plugins/geometry_checker/utils/qgsgeometrycheckerutils.cpp b/src/plugins/geometry_checker/utils/qgsgeometrycheckerutils.cpp
@@ -30,71 +30,106 @@ namespace QgsGeometryCheckerUtils
     , mFeature( feature )
     , mLayerToMapUnits( pool->getLayerToMapUnits() )
     , mLayerToMapTransform( pool->getLayerToMapTransform() )
-    , mClonedGeometry( false )
     , mMapCrs( useMapCrs )
   {
-    mGeometry = feature.geometry().geometry();
+    mGeometry = feature.geometry().geometry()->clone();
     if ( useMapCrs && !mLayerToMapTransform.isShortCircuited() )
     {
-      mClonedGeometry = true;
-      mGeometry = mGeometry->clone();
       mGeometry->transform( mLayerToMapTransform );
     }
   }
   LayerFeature::~LayerFeature()
   {
-    if ( mClonedGeometry )
-    {
-      delete mGeometry;
-    }
+    delete mGeometry;
   }
 
 /////////////////////////////////////////////////////////////////////////////
 
-  LayerFeatures::iterator::iterator( const QList<QString>::iterator &layerIt, const QgsFeatureIds::const_iterator &featureIt, const QgsFeature &feature, LayerFeatures *parent )
+  LayerFeatures::iterator::iterator( const QList<QString>::iterator &layerIt, LayerFeatures *parent )
     : mLayerIt( layerIt )
-    , mFeatureIt( featureIt )
-    , mFeature( feature )
+    , mFeatureIt( QgsFeatureIds::const_iterator() )
     , mParent( parent )
-    , mCurrentFeature( LayerFeature( parent->mFeaturePools[ * layerIt], feature, parent->mUseMapCrs ) )
   {
+    nextLayerFeature( true );
+  }
+  LayerFeatures::iterator::~iterator()
+  {
+    delete mCurrentFeature;
   }
 
   const LayerFeatures::iterator &LayerFeatures::iterator::operator++()
   {
-    if ( nextFeature() )
+    nextLayerFeature( false );
+    return *this;
+  }
+  bool LayerFeatures::iterator::nextLayerFeature( bool begin )
+  {
+    if ( !begin && nextFeature( false ) )
     {
-      return *this;
+      return true;
     }
-    do
+    while ( nextLayer( begin ) )
+    {
+      begin = false;
+      if ( nextFeature( true ) )
+      {
+        return true;
+      }
+    }
+    // End
+    mFeatureIt = QgsFeatureIds::const_iterator();
+    delete mCurrentFeature;
+    mCurrentFeature = 0;
+    return false;
+  }
+
+  bool LayerFeatures::iterator::nextLayer( bool begin )
+  {
+    if ( !begin )
     {
       ++mLayerIt;
-      mFeatureIt = mParent->mFeatureIds[*mLayerIt].begin();
-      if ( mParent->mGeometryTypes.contains( mParent->mFeaturePools[*mLayerIt]->getLayer()->geometryType() ) && nextFeature() )
+    }
+    while ( true )
+    {
+      if ( mLayerIt == mParent->mLayerIds.end() )
       {
-        return *this;
+        break;
       }
+      if ( mParent->mGeometryTypes.contains( mParent->mFeaturePools[*mLayerIt]->getLayer()->geometryType() ) )
+      {
+        mFeatureIt = mParent->mFeatureIds[*mLayerIt].begin();
+        return true;
+      }
+      ++mLayerIt;
     }
-    while ( mLayerIt != mParent->mLayerIds.end() );
-    return *this;
+    return false;
   }
 
-  bool LayerFeatures::iterator::nextFeature()
+  bool LayerFeatures::iterator::nextFeature( bool begin )
   {
     QgsFeaturePool *featurePool = mParent->mFeaturePools[*mLayerIt];
     const QgsFeatureIds &featureIds = mParent->mFeatureIds[*mLayerIt];
-    do
+    if ( !begin )
     {
       ++mFeatureIt;
+    }
+    while ( true )
+    {
+      if ( mFeatureIt == featureIds.end() )
+      {
+        break;
+      }
       if ( mParent->mProgressCounter )
         mParent->mProgressCounter->fetchAndAddRelaxed( 1 );
-      if ( featurePool->get( *mFeatureIt, mFeature ) )
+      QgsFeature feature;
+      if ( featurePool->get( *mFeatureIt, feature ) )
       {
-        mCurrentFeature = LayerFeature( mParent->mFeaturePools[*mLayerIt], mFeature, mParent->mUseMapCrs );
+        delete mCurrentFeature;
+        mCurrentFeature = new LayerFeature( mParent->mFeaturePools[*mLayerIt], feature, mParent->mUseMapCrs );
         return true;
       }
+      ++mFeatureIt;
     }
-    while ( mFeatureIt != featureIds.end() );
     return false;
   }
 
@@ -135,29 +170,6 @@ namespace QgsGeometryCheckerUtils
     }
   }
 
-  LayerFeatures::iterator LayerFeatures::begin()
-  {
-    for ( auto layerIt = mLayerIds.begin(), layerItEnd = mLayerIds.end(); layerIt != layerItEnd; ++layerIt )
-    {
-      if ( !mGeometryTypes.contains( mFeaturePools[*layerIt]->getLayer()->geometryType() ) )
-      {
-        continue;
-      }
-      const QgsFeatureIds &featureIds = mFeatureIds[*layerIt];
-      for ( auto featureIt = featureIds.begin(), featureItEnd = featureIds.end(); featureIt != featureItEnd; ++featureIt )
-      {
-        if ( mProgressCounter )
-          mProgressCounter->fetchAndAddRelaxed( 1 );
-        QgsFeature feature;
-        if ( mFeaturePools[*layerIt]->get( *featureIt, feature ) )
-        {
-          return iterator( layerIt, featureIt, feature, this );
-        }
-      }
-    }
-    return end();
-  }
-
   /////////////////////////////////////////////////////////////////////////////
 
   QSharedPointer<QgsGeometryEngine> createGeomEngine( const QgsAbstractGeometry *geometry, double tolerance )

diff --git a/src/plugins/geometry_checker/utils/qgsgeometrycheckerutils.h b/src/plugins/geometry_checker/utils/qgsgeometrycheckerutils.h
@@ -46,7 +46,6 @@ namespace QgsGeometryCheckerUtils
       double mLayerToMapUnits;
       QgsCoordinateTransform mLayerToMapTransform;
       QgsAbstractGeometry *mGeometry = nullptr;
-      bool mClonedGeometry = false;
       bool mMapCrs;
   };
 
@@ -64,26 +63,25 @@ namespace QgsGeometryCheckerUtils
       class iterator
       {
         public:
-          iterator( const QList<QString>::iterator &layerIt, const QgsFeatureIds::const_iterator &featureIt, const QgsFeature &feature, LayerFeatures *parent );
+          iterator( const QList<QString>::iterator &layerIt, LayerFeatures *parent );
+          ~iterator();
           const iterator &operator++();
           iterator operator++( int ) { iterator tmp( *this ); ++*this; return tmp; }
-          const LayerFeature &operator*() const { return mCurrentFeature; }
+          const LayerFeature &operator*() const { Q_ASSERT( mCurrentFeature ); return *mCurrentFeature; }
           bool operator!=( const iterator &other ) { return mLayerIt != other.mLayerIt || mFeatureIt != other.mFeatureIt; }
 
         private:
-          bool nextFeature();
+          bool nextLayerFeature( bool begin );
+          bool nextLayer( bool begin );
+          bool nextFeature( bool begin );
           QList<QString>::iterator mLayerIt;
           QgsFeatureIds::const_iterator mFeatureIt;
-          QgsFeature mFeature;
           LayerFeatures *mParent;
-          LayerFeature mCurrentFeature;
+          LayerFeature *mCurrentFeature = nullptr;
       };
 
-      iterator begin();
-      iterator end()
-      {
-        return iterator( mLayerIds.end(), mFeatureIds[mLayerIds.back()].end(), QgsFeature(), this );
-      }
+      iterator begin() { return iterator( mLayerIds.begin(), this ); }
+      iterator end() { return iterator( mLayerIds.end(), this ); }
     private:
       QMap<QString, QgsFeaturePool *> mFeaturePools;
       QMap<QString, QgsFeatureIds> mFeatureIds;