Address review, deduplicate code

qgis · Aug 30, 2021 · d42dab9 · d42dab9
1 parent a3db48a
commit d42dab9
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 65 deletions.
diff --git a/src/app/qgsstatisticalsummarydockwidget.cpp b/src/app/qgsstatisticalsummarydockwidget.cpp
@@ -15,6 +15,7 @@
 
 #include "qgisapp.h"
 #include "qgsclipboard.h"
+#include "qgsexpressionutils.h"
 #include "qgsmapcanvas.h"
 #include "qgsproject.h"
 #include "qgssettings.h"
@@ -163,40 +164,23 @@ void QgsStatisticalSummaryDockWidget::refreshStatistics()
   {
     QgsExpressionContext context;
     context.appendScopes( QgsExpressionContextUtils::globalProjectLayerScopes( mLayer ) );
-    QgsExpression expression( mFieldExpressionWidget->expression() );
-    QgsFeatureRequest request = QgsFeatureRequest();
-    request.setFlags( ( expression.needsGeometry() ) ?
-                      QgsFeatureRequest::NoFlags :
-                      QgsFeatureRequest::NoGeometry );
-    request.setLimit( 10 );
-    request.setExpressionContext( context );
-
-
-    QgsFeature f;
-    QgsFeatureIterator it = mSelectedOnlyCheckBox->isChecked() ? mLayer->getSelectedFeatures( request ) : mLayer->getFeatures( request );
-    bool hasFeature = it.nextFeature( f );
-    while ( hasFeature )
+    QgsFeatureRequest request;
+    if ( mSelectedOnlyCheckBox->isChecked() )
+      request.setFilterFids( mLayer->selectedFeatureIds() );
+
+    std::tuple<QVariant::Type, int> returnType = QgsExpressionUtils::determineResultType( mFieldExpressionWidget->expression(), mLayer, request, context );
+    switch ( std::get<0>( returnType ) )
     {
-      context.setFeature( f );
-      const QVariant v = expression.evaluate( &context );
-      if ( !v.isNull() )
-      {
-        switch ( v.type() )
-        {
-          case QVariant::String:
-            mFieldType = DataType::String;
-            break;
-          case QVariant::Date:
-          case QVariant::DateTime:
-            mFieldType = DataType::DateTime;
-            break;
-          default:
-            mFieldType = DataType::Numeric;
-            break;
-        }
+      case QVariant::String:
+        mFieldType = DataType::String;
+        break;
+      case QVariant::Date:
+      case QVariant::DateTime:
+        mFieldType = DataType::DateTime;
+        break;
+      default:
+        mFieldType = DataType::Numeric;
         break;
-      }
-      hasFeature = it.nextFeature( f );
     }
   }
 

diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt
@@ -1232,6 +1232,7 @@ set(QGIS_CORE_HDRS
   expression/qgsexpressionfunction.h
   expression/qgsexpressionnode.h
   expression/qgsexpressionnodeimpl.h
+  expression/qgsexpressionutils.h
 
   fieldformatter/qgscheckboxfieldformatter.h
   fieldformatter/qgsdatetimefieldformatter.h

diff --git a/src/core/expression/qgsexpressionutils.cpp b/src/core/expression/qgsexpressionutils.cpp
@@ -37,4 +37,33 @@ QgsExpressionUtils::TVL QgsExpressionUtils::NOT[3] = { True, False, Unknown };
 
 ///@endcond
 
+std::tuple<QVariant::Type, int> QgsExpressionUtils::determineResultType( const QString &expression, const QgsVectorLayer *layer, QgsFeatureRequest request, QgsExpressionContext context, bool *foundFeatures )
+{
+  QgsExpression exp( expression );
+  request.setFlags( ( exp.needsGeometry() ) ?
+                    QgsFeatureRequest::NoFlags :
+                    QgsFeatureRequest::NoGeometry );
+  request.setLimit( 10 );
+  request.setExpressionContext( context );
+
+  QVariant value;
+  QgsFeature f;
+  QgsFeatureIterator it = layer->getFeatures( request );
+  bool hasFeature = it.nextFeature( f );
+  if ( foundFeatures )
+    *foundFeatures = hasFeature;
+  while ( hasFeature )
+  {
+    context.setFeature( f );
+    const QVariant value = exp.evaluate( &context );
+    if ( !value.isNull() )
+    {
+      return std::make_tuple( value.type(), value.userType() );
+    }
+    hasFeature = it.nextFeature( f );
+  }
+  value = QVariant();
+  return std::make_tuple( value.type(), value.userType() );
+}
+
 
diff --git a/src/core/expression/qgsexpressionutils.h b/src/core/expression/qgsexpressionutils.h
@@ -38,13 +38,19 @@
 #define FEAT_FROM_CONTEXT(c, f) if ( !(c) || !( c )->hasFeature() ) return QVariant(); \
   QgsFeature f = ( c )->feature();
 
-///////////////////////////////////////////////
-// three-value logic
-
-/// @cond PRIVATE
-class QgsExpressionUtils
+/**
+ * \ingroup core
+ * \class QgsExpressionUtils
+ * \brief A set of expression-related functions
+ * \since QGIS 3.22
+ */
+
+class CORE_EXPORT QgsExpressionUtils
 {
   public:
+/// @cond PRIVATE
+///////////////////////////////////////////////
+// three-value logic
     enum TVL
     {
       False,
@@ -491,8 +497,20 @@ class QgsExpressionUtils
         return value.toString();
       }
     }
+/// @endcond
+
+    /**
+     * Returns a value type and user type for a given expression.
+     * \param expression An expression string.
+     * \param layer A vector layer from which the expression will be executed against.
+     * \param request A feature request object.
+     * \param context An expression context object.
+     * \param foundFeatures An optional boolean parameter that will be set when features are found.
+     * \since QGIS 3.22
+     */
+    static std::tuple<QVariant::Type, int> determineResultType( const QString &expression, const QgsVectorLayer *layer, QgsFeatureRequest request = QgsFeatureRequest(), QgsExpressionContext context = QgsExpressionContext(), bool *foundFeatures = nullptr );
+
 };
 
-/// @endcond
 
 #endif // QGSEXPRESSIONUTILS_H
diff --git a/src/core/qgsaggregatecalculator.cpp b/src/core/qgsaggregatecalculator.cpp
@@ -16,6 +16,7 @@
  ***************************************************************************/
 
 #include "qgsaggregatecalculator.h"
+#include "qgsexpressionutils.h"
 #include "qgsfeature.h"
 #include "qgsfeaturerequest.h"
 #include "qgsfeatureiterator.h"
@@ -116,39 +117,19 @@ QVariant QgsAggregateCalculator::calculate( QgsAggregateCalculator::Aggregate ag
     }
     else
     {
-      // evaluate first feature, check result type
-      QgsFeatureRequest testRequest( request );
-      testRequest.setLimit( 10 );
-      QgsFeature f;
-      QgsFeatureIterator fit = mLayer->getFeatures( testRequest );
-      if ( !fit.nextFeature( f ) )
+      // check expression result type
+      bool foundFeatures = false;
+      std::tuple<QVariant::Type, int> returnType = QgsExpressionUtils::determineResultType( fieldOrExpression, mLayer, request, *context, &foundFeatures );
+      if ( !foundFeatures )
       {
-        //no matching features
         if ( ok )
           *ok = true;
         return defaultValue( aggregate );
       }
 
-      bool hasFeature = true;
-      bool foundType = false;
-      while ( hasFeature && !foundType )
-      {
-        if ( context )
-          context->setFeature( f );
-        const QVariant v = expression->evaluate( context );
-        if ( !v.isNull() )
-        {
-          resultType = v.type();
-          userType = v.userType();
-          foundType = true;
-        }
-        else
-        {
-          hasFeature = fit.nextFeature( f );
-        }
-      }
-
-      if ( !foundType )
+      resultType = std::get<0>( returnType );
+      userType = std::get<1>( returnType );
+      if ( resultType == QVariant::Invalid )
       {
         QVariant v;
         switch ( aggregate )