Skip to content

Commit 647f322

Browse files
nyalldawsonnyalldawson
committed
Consolidate all qvariant sort methods to use qgsVariantLessThan,
make sure qgsVariantLessThan incorporates all functionality from other duplicate implementations, and add tests (fixes #14671)
1 parent 7290e8c commit 647f322

11 files changed

+154
-234
lines changed

‎python/core/qgis.sip

+11
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,17 @@ class QGis
243243
static double SCALE_PRECISION;
244244
};
245245

246+
//! Compares two QVariant values and returns whether the first is less than the second.
247+
//! Useful for sorting lists of variants, correctly handling sorting of the various
248+
//! QVariant data types (such as strings, numeric values, dates and times)
249+
//! @see qgsVariantGreaterThan()
250+
bool qgsVariantLessThan( const QVariant& lhs, const QVariant& rhs );
251+
252+
//! Compares two QVariant values and returns whether the first is greater than the second.
253+
//! Useful for sorting lists of variants, correctly handling sorting of the various
254+
//! QVariant data types (such as strings, numeric values, dates and times)
255+
//! @see qgsVariantLessThan()
256+
bool qgsVariantGreaterThan( const QVariant& lhs, const QVariant& rhs );
246257

247258
/** Wkt string that represents a geographic coord sys
248259
* @note added to replace GEOWkt

‎src/core/composer/qgsatlascomposition.cpp

+3-16
Original file line numberDiff line numberDiff line change
@@ -166,23 +166,10 @@ class FieldSorter
166166

167167
bool operator()( const QPair< QgsFeatureId, QString > & id1, const QPair< QgsFeatureId, QString >& id2 )
168168
{
169-
bool result = true;
170-
171-
if ( mKeys[ id1.first ].type() == QVariant::Int )
172-
{
173-
result = mKeys[ id1.first ].toInt() < mKeys[ id2.first ].toInt();
174-
}
175-
else if ( mKeys[ id1.first ].type() == QVariant::Double )
176-
{
177-
result = mKeys[ id1.first ].toDouble() < mKeys[ id2.first ].toDouble();
178-
}
179-
else if ( mKeys[ id1.first ].type() == QVariant::String )
180-
{
181-
result = ( QString::localeAwareCompare( mKeys[ id1.first ].toString(), mKeys[ id2.first ].toString() ) < 0 );
182-
}
183-
184-
return mAscending ? result : !result;
169+
return mAscending ? qgsVariantLessThan( mKeys.value( id1.first ), mKeys.value( id2.first ) )
170+
: qgsVariantGreaterThan( mKeys.value( id1.first ), mKeys.value( id2.first ) );
185171
}
172+
186173
private:
187174
QgsAtlasComposition::SorterKeys& mKeys;
188175
bool mAscending;

‎src/core/composer/qgscomposerattributetable.cpp

+2-54
Original file line numberDiff line numberDiff line change
@@ -32,60 +32,8 @@ QgsComposerAttributeTableCompare::QgsComposerAttributeTableCompare()
3232

3333
bool QgsComposerAttributeTableCompare::operator()( const QgsAttributeMap& m1, const QgsAttributeMap& m2 )
3434
{
35-
QVariant v1 = m1[mCurrentSortColumn];
36-
QVariant v2 = m2[mCurrentSortColumn];
37-
38-
bool less = false;
39-
40-
//sort null values first
41-
if ( v1.isNull() && v2.isNull() )
42-
{
43-
less = false;
44-
}
45-
else if ( v1.isNull() )
46-
{
47-
less = true;
48-
}
49-
else if ( v2.isNull() )
50-
{
51-
less = false;
52-
}
53-
else
54-
{
55-
//otherwise sort by converting to corresponding type and comparing
56-
switch ( v1.type() )
57-
{
58-
case QVariant::Int:
59-
case QVariant::UInt:
60-
case QVariant::LongLong:
61-
case QVariant::ULongLong:
62-
less = v1.toLongLong() < v2.toLongLong();
63-
break;
64-
65-
case QVariant::Double:
66-
less = v1.toDouble() < v2.toDouble();
67-
break;
68-
69-
case QVariant::Date:
70-
less = v1.toDate() < v2.toDate();
71-
break;
72-
73-
case QVariant::DateTime:
74-
less = v1.toDateTime() < v2.toDateTime();
75-
break;
76-
77-
case QVariant::Time:
78-
less = v1.toTime() < v2.toTime();
79-
break;
80-
81-
default:
82-
//use locale aware compare for strings
83-
less = v1.toString().localeAwareCompare( v2.toString() ) < 0;
84-
break;
85-
}
86-
}
87-
88-
return ( mAscending ? less : !less );
35+
return ( mAscending ? qgsVariantLessThan( m1[mCurrentSortColumn], m2[mCurrentSortColumn] )
36+
: qgsVariantGreaterThan( m1[mCurrentSortColumn], m2[mCurrentSortColumn] ) );
8937
}
9038

9139

‎src/core/composer/qgscomposerattributetablev2.cpp

+2-54
Original file line numberDiff line numberDiff line change
@@ -37,60 +37,8 @@ QgsComposerAttributeTableCompareV2::QgsComposerAttributeTableCompareV2()
3737

3838
bool QgsComposerAttributeTableCompareV2::operator()( const QgsComposerTableRow& m1, const QgsComposerTableRow& m2 )
3939
{
40-
QVariant v1 = m1[mCurrentSortColumn];
41-
QVariant v2 = m2[mCurrentSortColumn];
42-
43-
bool less = false;
44-
45-
//sort null values first
46-
if ( v1.isNull() && v2.isNull() )
47-
{
48-
less = false;
49-
}
50-
else if ( v1.isNull() )
51-
{
52-
less = true;
53-
}
54-
else if ( v2.isNull() )
55-
{
56-
less = false;
57-
}
58-
else
59-
{
60-
//otherwise sort by converting to corresponding type and comparing
61-
switch ( v1.type() )
62-
{
63-
case QVariant::Int:
64-
case QVariant::UInt:
65-
case QVariant::LongLong:
66-
case QVariant::ULongLong:
67-
less = v1.toLongLong() < v2.toLongLong();
68-
break;
69-
70-
case QVariant::Double:
71-
less = v1.toDouble() < v2.toDouble();
72-
break;
73-
74-
case QVariant::Date:
75-
less = v1.toDate() < v2.toDate();
76-
break;
77-
78-
case QVariant::DateTime:
79-
less = v1.toDateTime() < v2.toDateTime();
80-
break;
81-
82-
case QVariant::Time:
83-
less = v1.toTime() < v2.toTime();
84-
break;
85-
86-
default:
87-
//use locale aware compare for strings
88-
less = v1.toString().localeAwareCompare( v2.toString() ) < 0;
89-
break;
90-
}
91-
}
92-
93-
return ( mAscending ? less : !less );
40+
return ( mAscending ? qgsVariantLessThan( m1[mCurrentSortColumn], m2[mCurrentSortColumn] )
41+
: qgsVariantGreaterThan( m1[mCurrentSortColumn], m2[mCurrentSortColumn] ) );
9442
}
9543

9644
//

‎src/core/qgis.cpp

+41
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,14 @@ void qgsFree( void *ptr )
262262

263263
bool qgsVariantLessThan( const QVariant& lhs, const QVariant& rhs )
264264
{
265+
// invalid < NULL < any value
266+
if ( !lhs.isValid() )
267+
return rhs.isValid();
268+
else if ( lhs.isNull() )
269+
return rhs.isValid() && !rhs.isNull();
270+
else if ( !rhs.isValid() || rhs.isNull() )
271+
return false;
272+
265273
switch ( lhs.type() )
266274
{
267275
case QVariant::Int:
@@ -282,6 +290,39 @@ bool qgsVariantLessThan( const QVariant& lhs, const QVariant& rhs )
282290
return lhs.toTime() < rhs.toTime();
283291
case QVariant::DateTime:
284292
return lhs.toDateTime() < rhs.toDateTime();
293+
case QVariant::Bool:
294+
return lhs.toBool() < rhs.toBool();
295+
296+
case QVariant::List:
297+
{
298+
const QList<QVariant> &lhsl = lhs.toList();
299+
const QList<QVariant> &rhsl = rhs.toList();
300+
301+
int i, n = qMin( lhsl.size(), rhsl.size() );
302+
for ( i = 0; i < n && lhsl[i].type() == rhsl[i].type() && lhsl[i].isNull() == rhsl[i].isNull() && lhsl[i] == rhsl[i]; i++ )
303+
;
304+
305+
if ( i == n )
306+
return lhsl.size() < rhsl.size();
307+
else
308+
return qgsVariantLessThan( lhsl[i], rhsl[i] );
309+
}
310+
311+
case QVariant::StringList:
312+
{
313+
const QStringList &lhsl = lhs.toStringList();
314+
const QStringList &rhsl = rhs.toStringList();
315+
316+
int i, n = qMin( lhsl.size(), rhsl.size() );
317+
for ( i = 0; i < n && lhsl[i] == rhsl[i]; i++ )
318+
;
319+
320+
if ( i == n )
321+
return lhsl.size() < rhsl.size();
322+
else
323+
return lhsl[i] < rhsl[i];
324+
}
325+
285326
default:
286327
return QString::localeAwareCompare( lhs.toString(), rhs.toString() ) < 0;
287328
}

‎src/core/qgis.h

+20-15
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,9 @@ template<class Object> inline QgsSignalBlocker<Object> whileBlocking( Object* ob
330330
return QgsSignalBlocker<Object>( object );
331331
}
332332

333-
//
334-
// return a string representation of a double
335-
//
333+
//! Returns a string representation of a double
334+
//! @param a double value
335+
//! @param precision number of decimal places to retain
336336
inline QString qgsDoubleToString( double a, int precision = 17 )
337337
{
338338
if ( precision )
@@ -341,18 +341,17 @@ inline QString qgsDoubleToString( double a, int precision = 17 )
341341
return QString::number( a, 'f', precision );
342342
}
343343

344-
//
345-
// compare two doubles (but allow some difference)
346-
//
344+
//! Compare two doubles (but allow some difference)
345+
//! @param a first double
346+
//! @param b second double
347+
//! @param epsilon maximum difference allowable between doubles
347348
inline bool qgsDoubleNear( double a, double b, double epsilon = 4 * DBL_EPSILON )
348349
{
349350
const double diff = a - b;
350351
return diff > -epsilon && diff <= epsilon;
351352
}
352353

353-
//
354-
// compare two doubles using specified number of significant digits
355-
//
354+
//! Compare two doubles using specified number of significant digits
356355
inline bool qgsDoubleNearSig( double a, double b, int significantDigits = 10 )
357356
{
358357
// The most simple would be to print numbers as %.xe and compare as strings
@@ -368,17 +367,23 @@ inline bool qgsDoubleNearSig( double a, double b, int significantDigits = 10 )
368367
qRound( ar * pow( 10.0, significantDigits ) ) == qRound( br * pow( 10.0, significantDigits ) );
369368
}
370369

371-
//
372-
// a round function which returns a double to guard against overflows
373-
//
370+
//! A round function which returns a double to guard against overflows
374371
inline double qgsRound( double x )
375372
{
376373
return x < 0.0 ? std::ceil( x - 0.5 ) : std::floor( x + 0.5 );
377374
}
378375

379-
bool qgsVariantLessThan( const QVariant& lhs, const QVariant& rhs );
380-
381-
bool qgsVariantGreaterThan( const QVariant& lhs, const QVariant& rhs );
376+
//! Compares two QVariant values and returns whether the first is less than the second.
377+
//! Useful for sorting lists of variants, correctly handling sorting of the various
378+
//! QVariant data types (such as strings, numeric values, dates and times)
379+
//! @see qgsVariantGreaterThan()
380+
CORE_EXPORT bool qgsVariantLessThan( const QVariant& lhs, const QVariant& rhs );
381+
382+
//! Compares two QVariant values and returns whether the first is greater than the second.
383+
//! Useful for sorting lists of variants, correctly handling sorting of the various
384+
//! QVariant data types (such as strings, numeric values, dates and times)
385+
//! @see qgsVariantLessThan()
386+
CORE_EXPORT bool qgsVariantGreaterThan( const QVariant& lhs, const QVariant& rhs );
382387

383388
CORE_EXPORT QString qgsVsiPrefix( const QString& path );
384389

‎src/core/symbology-ng/qgssymbollayerv2utils.cpp

-34
Original file line numberDiff line numberDiff line change
@@ -3642,40 +3642,6 @@ void QgsSymbolLayerV2Utils::premultiplyColor( QColor &rgb, int alpha )
36423642
}
36433643
}
36443644

3645-
#if 0
3646-
static bool _QVariantLessThan( const QVariant& lhs, const QVariant& rhs )
3647-
{
3648-
switch ( lhs.type() )
3649-
{
3650-
case QVariant::Int:
3651-
return lhs.toInt() < rhs.toInt();
3652-
case QVariant::UInt:
3653-
return lhs.toUInt() < rhs.toUInt();
3654-
case QVariant::LongLong:
3655-
return lhs.toLongLong() < rhs.toLongLong();
3656-
case QVariant::ULongLong:
3657-
return lhs.toULongLong() < rhs.toULongLong();
3658-
case QVariant::Double:
3659-
return lhs.toDouble() < rhs.toDouble();
3660-
case QVariant::Char:
3661-
return lhs.toChar() < rhs.toChar();
3662-
case QVariant::Date:
3663-
return lhs.toDate() < rhs.toDate();
3664-
case QVariant::Time:
3665-
return lhs.toTime() < rhs.toTime();
3666-
case QVariant::DateTime:
3667-
return lhs.toDateTime() < rhs.toDateTime();
3668-
default:
3669-
return QString::localeAwareCompare( lhs.toString(), rhs.toString() ) < 0;
3670-
}
3671-
}
3672-
3673-
static bool _QVariantGreaterThan( const QVariant& lhs, const QVariant& rhs )
3674-
{
3675-
return ! _QVariantLessThan( lhs, rhs );
3676-
}
3677-
#endif
3678-
36793645
void QgsSymbolLayerV2Utils::sortVariantList( QList<QVariant>& list, Qt::SortOrder order )
36803646
{
36813647
if ( order == Qt::AscendingOrder )

‎src/gui/attributetable/qgsattributetablefiltermodel.cpp

+3-33
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include <QItemSelectionModel>
1717

18+
#include "qgis.h"
1819
#include "qgsattributetablefiltermodel.h"
1920
#include "qgsattributetablemodel.h"
2021
#include "qgsvectorlayer.h"
@@ -56,39 +57,8 @@ bool QgsAttributeTableFilterModel::lessThan( const QModelIndex &left, const QMod
5657
}
5758
}
5859

59-
60-
QVariant leftData = left.data( QgsAttributeTableModel::SortRole );
61-
QVariant rightData = right.data( QgsAttributeTableModel::SortRole );
62-
63-
if ( leftData.isNull() )
64-
return true;
65-
66-
if ( rightData.isNull() )
67-
return false;
68-
69-
switch ( leftData.type() )
70-
{
71-
case QVariant::Int:
72-
case QVariant::UInt:
73-
case QVariant::LongLong:
74-
case QVariant::ULongLong:
75-
return leftData.toLongLong() < rightData.toLongLong();
76-
77-
case QVariant::Double:
78-
return leftData.toDouble() < rightData.toDouble();
79-
80-
case QVariant::Date:
81-
return leftData.toDate() < rightData.toDate();
82-
83-
case QVariant::Time:
84-
return leftData.toTime() < rightData.toTime();
85-
86-
case QVariant::DateTime:
87-
return leftData.toDateTime() < rightData.toDateTime();
88-
89-
default:
90-
return leftData.toString().localeAwareCompare( rightData.toString() ) < 0;
91-
}
60+
return qgsVariantLessThan( left.data( QgsAttributeTableModel::SortRole ),
61+
right.data( QgsAttributeTableModel::SortRole ) );
9262
}
9363

9464
void QgsAttributeTableFilterModel::sort( int column, Qt::SortOrder order )

0 commit comments

Comments
 (0)