Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Consolidate all qvariant sort methods to use qgsVariantLessThan,
make sure qgsVariantLessThan incorporates all functionality from
other duplicate implementations, and add tests

(fixes #14671)

(cherry-picked from 647f32)
  • Loading branch information
nyalldawson committed Apr 26, 2016
1 parent 6663fd2 commit e657f61
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 222 deletions.
11 changes: 11 additions & 0 deletions python/core/qgis.sip
Expand Up @@ -233,6 +233,17 @@ class QGis
static double DEFAULT_HIGHLIGHT_MIN_WIDTH_MM;
};

//! Compares two QVariant values and returns whether the first is less than the second.
//! Useful for sorting lists of variants, correctly handling sorting of the various
//! QVariant data types (such as strings, numeric values, dates and times)
//! @see qgsVariantGreaterThan()
bool qgsVariantLessThan( 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
//! QVariant data types (such as strings, numeric values, dates and times)
//! @see qgsVariantLessThan()
bool qgsVariantGreaterThan( const QVariant& lhs, const QVariant& rhs );

/** Wkt string that represents a geographic coord sys
* @note added to replace GEOWkt
Expand Down
19 changes: 3 additions & 16 deletions src/core/composer/qgsatlascomposition.cpp
Expand Up @@ -166,23 +166,10 @@ class FieldSorter

bool operator()( const QPair< QgsFeatureId, QString > & id1, const QPair< QgsFeatureId, QString >& id2 )
{
bool result = true;

if ( mKeys[ id1.first ].type() == QVariant::Int )
{
result = mKeys[ id1.first ].toInt() < mKeys[ id2.first ].toInt();
}
else if ( mKeys[ id1.first ].type() == QVariant::Double )
{
result = mKeys[ id1.first ].toDouble() < mKeys[ id2.first ].toDouble();
}
else if ( mKeys[ id1.first ].type() == QVariant::String )
{
result = ( QString::localeAwareCompare( mKeys[ id1.first ].toString(), mKeys[ id2.first ].toString() ) < 0 );
}

return mAscending ? result : !result;
return mAscending ? qgsVariantLessThan( mKeys.value( id1.first ), mKeys.value( id2.first ) )
: qgsVariantGreaterThan( mKeys.value( id1.first ), mKeys.value( id2.first ) );
}

private:
QgsAtlasComposition::SorterKeys& mKeys;
bool mAscending;
Expand Down
56 changes: 2 additions & 54 deletions src/core/composer/qgscomposerattributetable.cpp
Expand Up @@ -32,60 +32,8 @@ QgsComposerAttributeTableCompare::QgsComposerAttributeTableCompare()

bool QgsComposerAttributeTableCompare::operator()( const QgsAttributeMap& m1, const QgsAttributeMap& m2 )
{
QVariant v1 = m1[mCurrentSortColumn];
QVariant v2 = m2[mCurrentSortColumn];

bool less = false;

//sort null values first
if ( v1.isNull() && v2.isNull() )
{
less = false;
}
else if ( v1.isNull() )
{
less = true;
}
else if ( v2.isNull() )
{
less = false;
}
else
{
//otherwise sort by converting to corresponding type and comparing
switch ( v1.type() )
{
case QVariant::Int:
case QVariant::UInt:
case QVariant::LongLong:
case QVariant::ULongLong:
less = v1.toLongLong() < v2.toLongLong();
break;

case QVariant::Double:
less = v1.toDouble() < v2.toDouble();
break;

case QVariant::Date:
less = v1.toDate() < v2.toDate();
break;

case QVariant::DateTime:
less = v1.toDateTime() < v2.toDateTime();
break;

case QVariant::Time:
less = v1.toTime() < v2.toTime();
break;

default:
//use locale aware compare for strings
less = v1.toString().localeAwareCompare( v2.toString() ) < 0;
break;
}
}

return ( mAscending ? less : !less );
return ( mAscending ? qgsVariantLessThan( m1[mCurrentSortColumn], m2[mCurrentSortColumn] )
: qgsVariantGreaterThan( m1[mCurrentSortColumn], m2[mCurrentSortColumn] ) );
}


Expand Down
56 changes: 2 additions & 54 deletions src/core/composer/qgscomposerattributetablev2.cpp
Expand Up @@ -37,60 +37,8 @@ QgsComposerAttributeTableCompareV2::QgsComposerAttributeTableCompareV2()

bool QgsComposerAttributeTableCompareV2::operator()( const QgsComposerTableRow& m1, const QgsComposerTableRow& m2 )
{
QVariant v1 = m1[mCurrentSortColumn];
QVariant v2 = m2[mCurrentSortColumn];

bool less = false;

//sort null values first
if ( v1.isNull() && v2.isNull() )
{
less = false;
}
else if ( v1.isNull() )
{
less = true;
}
else if ( v2.isNull() )
{
less = false;
}
else
{
//otherwise sort by converting to corresponding type and comparing
switch ( v1.type() )
{
case QVariant::Int:
case QVariant::UInt:
case QVariant::LongLong:
case QVariant::ULongLong:
less = v1.toLongLong() < v2.toLongLong();
break;

case QVariant::Double:
less = v1.toDouble() < v2.toDouble();
break;

case QVariant::Date:
less = v1.toDate() < v2.toDate();
break;

case QVariant::DateTime:
less = v1.toDateTime() < v2.toDateTime();
break;

case QVariant::Time:
less = v1.toTime() < v2.toTime();
break;

default:
//use locale aware compare for strings
less = v1.toString().localeAwareCompare( v2.toString() ) < 0;
break;
}
}

return ( mAscending ? less : !less );
return ( mAscending ? qgsVariantLessThan( m1[mCurrentSortColumn], m2[mCurrentSortColumn] )
: qgsVariantGreaterThan( m1[mCurrentSortColumn], m2[mCurrentSortColumn] ) );
}

//
Expand Down
41 changes: 41 additions & 0 deletions src/core/qgis.cpp
Expand Up @@ -260,6 +260,14 @@ void qgsFree( void *ptr )

bool qgsVariantLessThan( const QVariant& lhs, const QVariant& rhs )
{
// invalid < NULL < any value
if ( !lhs.isValid() )
return rhs.isValid();
else if ( lhs.isNull() )
return rhs.isValid() && !rhs.isNull();
else if ( !rhs.isValid() || rhs.isNull() )
return false;

switch ( lhs.type() )
{
case QVariant::Int:
Expand All @@ -280,6 +288,39 @@ bool qgsVariantLessThan( const QVariant& lhs, const QVariant& rhs )
return lhs.toTime() < rhs.toTime();
case QVariant::DateTime:
return lhs.toDateTime() < rhs.toDateTime();
case QVariant::Bool:
return lhs.toBool() < rhs.toBool();

case QVariant::List:
{
const QList<QVariant> &lhsl = lhs.toList();
const QList<QVariant> &rhsl = rhs.toList();

int i, n = qMin( 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++ )
;

if ( i == n )
return lhsl.size() < rhsl.size();
else
return qgsVariantLessThan( lhsl[i], rhsl[i] );
}

case QVariant::StringList:
{
const QStringList &lhsl = lhs.toStringList();
const QStringList &rhsl = rhs.toStringList();

int i, n = qMin( lhsl.size(), rhsl.size() );
for ( i = 0; i < n && lhsl[i] == rhsl[i]; i++ )
;

if ( i == n )
return lhsl.size() < rhsl.size();
else
return lhsl[i] < rhsl[i];
}

default:
return QString::localeAwareCompare( lhs.toString(), rhs.toString() ) < 0;
}
Expand Down
14 changes: 11 additions & 3 deletions src/core/qgis.h
Expand Up @@ -314,9 +314,17 @@ inline double qgsRound( double x )
return x < 0.0 ? std::ceil( x - 0.5 ) : std::floor( x + 0.5 );
}

bool qgsVariantLessThan( const QVariant& lhs, const QVariant& rhs );

bool qgsVariantGreaterThan( const QVariant& lhs, const QVariant& rhs );
//! Compares two QVariant values and returns whether the first is less than the second.
//! Useful for sorting lists of variants, correctly handling sorting of the various
//! QVariant data types (such as strings, numeric values, dates and times)
//! @see qgsVariantGreaterThan()
CORE_EXPORT bool qgsVariantLessThan( 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
//! QVariant data types (such as strings, numeric values, dates and times)
//! @see qgsVariantLessThan()
CORE_EXPORT bool qgsVariantGreaterThan( const QVariant& lhs, const QVariant& rhs );

CORE_EXPORT QString qgsVsiPrefix( const QString& path );

Expand Down
34 changes: 0 additions & 34 deletions src/core/symbology-ng/qgssymbollayerv2utils.cpp
Expand Up @@ -3590,40 +3590,6 @@ void QgsSymbolLayerV2Utils::premultiplyColor( QColor &rgb, int alpha )
}
}

#if 0
static bool _QVariantLessThan( const QVariant& lhs, const QVariant& rhs )
{
switch ( lhs.type() )
{
case QVariant::Int:
return lhs.toInt() < rhs.toInt();
case QVariant::UInt:
return lhs.toUInt() < rhs.toUInt();
case QVariant::LongLong:
return lhs.toLongLong() < rhs.toLongLong();
case QVariant::ULongLong:
return lhs.toULongLong() < rhs.toULongLong();
case QVariant::Double:
return lhs.toDouble() < rhs.toDouble();
case QVariant::Char:
return lhs.toChar() < rhs.toChar();
case QVariant::Date:
return lhs.toDate() < rhs.toDate();
case QVariant::Time:
return lhs.toTime() < rhs.toTime();
case QVariant::DateTime:
return lhs.toDateTime() < rhs.toDateTime();
default:
return QString::localeAwareCompare( lhs.toString(), rhs.toString() ) < 0;
}
}

static bool _QVariantGreaterThan( const QVariant& lhs, const QVariant& rhs )
{
return ! _QVariantLessThan( lhs, rhs );
}
#endif

void QgsSymbolLayerV2Utils::sortVariantList( QList<QVariant>& list, Qt::SortOrder order )
{
if ( order == Qt::AscendingOrder )
Expand Down
36 changes: 3 additions & 33 deletions src/gui/attributetable/qgsattributetablefiltermodel.cpp
Expand Up @@ -15,6 +15,7 @@

#include <QItemSelectionModel>

#include "qgis.h"
#include "qgsattributetablefiltermodel.h"
#include "qgsattributetablemodel.h"
#include "qgsvectorlayer.h"
Expand Down Expand Up @@ -56,39 +57,8 @@ bool QgsAttributeTableFilterModel::lessThan( const QModelIndex &left, const QMod
}
}


QVariant leftData = left.data( QgsAttributeTableModel::SortRole );
QVariant rightData = right.data( QgsAttributeTableModel::SortRole );

if ( leftData.isNull() )
return true;

if ( rightData.isNull() )
return false;

switch ( leftData.type() )
{
case QVariant::Int:
case QVariant::UInt:
case QVariant::LongLong:
case QVariant::ULongLong:
return leftData.toLongLong() < rightData.toLongLong();

case QVariant::Double:
return leftData.toDouble() < rightData.toDouble();

case QVariant::Date:
return leftData.toDate() < rightData.toDate();

case QVariant::Time:
return leftData.toTime() < rightData.toTime();

case QVariant::DateTime:
return leftData.toDateTime() < rightData.toDateTime();

default:
return leftData.toString().localeAwareCompare( rightData.toString() ) < 0;
}
return qgsVariantLessThan( left.data( QgsAttributeTableModel::SortRole ),
right.data( QgsAttributeTableModel::SortRole ) );
}

void QgsAttributeTableFilterModel::sort( int column, Qt::SortOrder order )
Expand Down
16 changes: 0 additions & 16 deletions src/gui/editorwidgets/qgsrelationreferencewidget.cpp
Expand Up @@ -34,22 +34,6 @@
#include "qgsvectorlayer.h"
#include "qgsattributetablemodel.h"

bool orderByLessThan( const QgsRelationReferenceWidget::ValueRelationItem& p1
, const QgsRelationReferenceWidget::ValueRelationItem& p2 )
{
switch ( p1.first.type() )
{
case QVariant::String:
return p1.first.toString() < p2.first.toString();

case QVariant::Double:
return p1.first.toDouble() < p2.first.toDouble();

default:
return p1.first.toInt() < p2.first.toInt();
}
}

QgsRelationReferenceWidget::QgsRelationReferenceWidget( QWidget* parent )
: QWidget( parent )
, mEditorContext( QgsAttributeEditorContext() )
Expand Down
15 changes: 3 additions & 12 deletions src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp
Expand Up @@ -15,6 +15,7 @@

#include "qgsvaluerelationwidgetwrapper.h"

#include "qgis.h"
#include "qgsfield.h"
#include "qgsmaplayerregistry.h"
#include "qgsvaluerelationwidgetfactory.h"
Expand All @@ -27,23 +28,13 @@
bool QgsValueRelationWidgetWrapper::orderByKeyLessThan( const QgsValueRelationWidgetWrapper::ValueRelationItem& p1
, const QgsValueRelationWidgetWrapper::ValueRelationItem& p2 )
{
switch ( p1.first.type() )
{
case QVariant::String:
return p1.first.toString() < p2.first.toString();

case QVariant::Double:
return p1.first.toDouble() < p2.first.toDouble();

default:
return p1.first.toInt() < p2.first.toInt();
}
return qgsVariantLessThan( p1.first, p2.first );
}

bool QgsValueRelationWidgetWrapper::orderByValueLessThan( const QgsValueRelationWidgetWrapper::ValueRelationItem& p1
, const QgsValueRelationWidgetWrapper::ValueRelationItem& p2 )
{
return p1.second < p2.second;
return qgsVariantLessThan( p1.second, p2.second );
}

QgsValueRelationWidgetWrapper::QgsValueRelationWidgetWrapper( QgsVectorLayer* vl, int fieldIdx, QWidget* editor, QWidget* parent )
Expand Down

0 comments on commit e657f61

Please sign in to comment.