Merge pull request #8082 from elpaso/qvariant-equal

Add qgsVariantEqual for null-aware comparison
qgis · Oct 1, 2018 · ff41882 · ff41882
2 parents fecf2eb + 5ed02a7
commit ff41882
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 7 deletions.
diff --git a/python/core/auto_generated/qgis.sip.in b/python/core/auto_generated/qgis.sip.in
@@ -187,6 +187,17 @@ QVariant data types (such as strings, numeric values, dates and times)
 .. seealso:: :py:func:`qgsVariantGreaterThan`
 %End
 
+bool qgsVariantEqual( const QVariant &lhs, const QVariant &rhs );
+%Docstring
+Compares two QVariant values and returns whether they are equal, NULL values are treated as equal.
+
+:param lhs: first value
+:param rhs: second value
+
+:return: true if values are equal
+%End
+
+
 bool qgsVariantGreaterThan( const QVariant &lhs, const QVariant &rhs );
 %Docstring
 Compares two QVariant values and returns whether the first is greater than the second.

diff --git a/src/core/qgis.cpp b/src/core/qgis.cpp
@@ -189,7 +189,7 @@ bool qgsVariantLessThan( const QVariant &lhs, const QVariant &rhs )
       const QList<QVariant> &rhsl = rhs.toList();
 
       int i, n = std::min( lhsl.size(), rhsl.size() );
-      for ( i = 0; i < n && lhsl[i].type() == rhsl[i].type() && lhsl[i].isNull() == rhsl[i].isNull() && lhsl[i] == rhsl[i]; i++ )
+      for ( i = 0; i < n && lhsl[i].type() == rhsl[i].type() && qgsVariantEqual( lhsl[i], rhsl[i] ); i++ )
         ;
 
       if ( i == n )
@@ -306,3 +306,8 @@ uint qHash( const QVariant &variant )
 
   return std::numeric_limits<uint>::max();
 }
+
+bool qgsVariantEqual( const QVariant &lhs, const QVariant &rhs )
+{
+  return lhs.isNull() == rhs.isNull() && lhs == rhs;
+}
diff --git a/src/core/qgis.h b/src/core/qgis.h
@@ -448,6 +448,16 @@ CORE_EXPORT qlonglong qgsPermissiveToLongLong( QString string, bool &ok );
  */
 CORE_EXPORT bool qgsVariantLessThan( const QVariant &lhs, const QVariant &rhs );
 
+/**
+ * Compares two QVariant values and returns whether they are equal, NULL values are treated as equal.
+ *
+ * \param lhs first value
+ * \param rhs second value
+ * \return true if values are equal
+ */
+CORE_EXPORT bool qgsVariantEqual( const QVariant &lhs, const QVariant &rhs );
+
+
 /**
  * Compares two QVariant values and returns whether the first is greater than the second.
  * Useful for sorting lists of variants, correctly handling sorting of the various

diff --git a/src/core/qgsfeaturefiltermodel.cpp b/src/core/qgsfeaturefiltermodel.cpp
@@ -533,7 +533,7 @@ QVariant QgsFeatureFilterModel::extraIdentifierValue() const
 
 void QgsFeatureFilterModel::setExtraIdentifierValue( const QVariant &extraIdentifierValue )
 {
-  if ( extraIdentifierValue == mExtraIdentifierValue && extraIdentifierValue.isNull() == mExtraIdentifierValue.isNull() && mExtraIdentifierValue.isValid() )
+  if ( qgsVariantEqual( extraIdentifierValue, mExtraIdentifierValue ) && mExtraIdentifierValue.isValid() )
     return;
 
   if ( mIsSettingExtraIdentifierValue )

diff --git a/src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp b/src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp
@@ -185,7 +185,7 @@ void QgsValueRelationWidgetWrapper::setValue( const QVariant &value )
     for ( int i = 0; i < mComboBox->count(); i++ )
     {
       QVariant v( mComboBox->itemData( i ) );
-      if ( v.isNull() == value.isNull() && v == value )
+      if ( qgsVariantEqual( v, value ) )
       {
         idx = i;
         break;

diff --git a/src/gui/qgsattributeform.cpp b/src/gui/qgsattributeform.cpp
@@ -357,9 +357,9 @@ bool QgsAttributeForm::saveEdits()
         int n = 0;
         for ( int i = 0; i < dst.count(); ++i )
         {
-          if ( ( dst.at( i ) == src.at( i ) && dst.at( i ).isNull() == src.at( i ).isNull() ) // If field is not changed...
-               || !dst.at( i ).isValid()                                     // or the widget returns invalid (== do not change)
-               || !fieldIsEditable( i ) )                           // or the field cannot be edited ...
+          if ( qgsVariantEqual( dst.at( i ), src.at( i ) ) // If field is not changed...
+               || !dst.at( i ).isValid()                 // or the widget returns invalid (== do not change)
+               || !fieldIsEditable( i ) )                // or the field cannot be edited ...
           {
             continue;
           }

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -2118,7 +2118,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
 
       if ( i == flist.size() )
       {
-        if ( v == defVal && defVal.isNull() == v.isNull() )
+        if ( qgsVariantEqual( v, defVal ) )
         {
           if ( defVal.isNull() )
           {

diff --git a/tests/src/core/testqgis.cpp b/tests/src/core/testqgis.cpp
@@ -45,6 +45,7 @@ class TestQgis : public QObject
     void qVariantCompare();
     void testQgsAsConst();
     void testQgsRound();
+    void testQgsVariantEqual();
 
   private:
     QString mReport;
@@ -359,6 +360,35 @@ void TestQgis::testQgsRound()
   QGSCOMPARENEAR( qgsRound( 9999999.87654321987654321, 14 ), 9999999.876543219876543, 0.00000000000001 );
 }
 
+void TestQgis::testQgsVariantEqual()
+{
+
+  // Invalid
+  QVERIFY( qgsVariantEqual( QVariant(), QVariant() ) );
+  QVERIFY( QVariant() == QVariant() );
+
+  // Zero
+  QVERIFY( qgsVariantEqual( QVariant( 0 ), QVariant( 0.0f ) ) );
+  QVERIFY( QVariant( 0 ) == QVariant( 0.0f ) );
+
+  // Double
+  QVERIFY( qgsVariantEqual( QVariant( 1.234 ), QVariant( 1.234 ) ) );
+
+  // This is what we actually wanted to fix with qgsVariantEqual
+  // zero != NULL
+  QVERIFY( ! qgsVariantEqual( QVariant( 0 ), QVariant( QVariant::Int ) ) );
+  QVERIFY( ! qgsVariantEqual( QVariant( 0 ), QVariant( QVariant::Double ) ) );
+  QVERIFY( ! qgsVariantEqual( QVariant( 0.0f ), QVariant( QVariant::Int ) ) );
+  QVERIFY( ! qgsVariantEqual( QVariant( 0.0f ), QVariant( QVariant::Double ) ) );
+  QVERIFY( QVariant( 0 ) == QVariant( QVariant::Int ) );
+
+  // NULL identities
+  QVERIFY( qgsVariantEqual( QVariant( QVariant::Int ), QVariant( QVariant::Int ) ) );
+  QVERIFY( qgsVariantEqual( QVariant( QVariant::Double ), QVariant( QVariant::Double ) ) );
+
+
+}
+
 
 QGSTEST_MAIN( TestQgis )
 #include "testqgis.moc"