Skip to content

Commit

Permalink
qgsgeometrycollection: Remove uneeded clearCache call in operator=
Browse files Browse the repository at this point in the history 
Indeed `QgsAbstractGeometry::operator=()` is already called. This
operator calls the `clear()` method which already clears the cache and
the existing geometries.
  • Loading branch information
@ptitjano @nyalldawson
ptitjano authored and nyalldawson committed Sep 5, 2024
1 parent 04b1dd7 commit 9fed0f1
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/core/geometry/qgsgeometrycollection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ QgsGeometryCollection::QgsGeometryCollection( const QgsGeometryCollection &c ):
}
}

// cppcheck-suppress operatorEqVarError
QgsGeometryCollection &QgsGeometryCollection::operator=( const QgsGeometryCollection &c )
{
if ( &c != this )
{
clearCache();
QgsAbstractGeometry::operator=( c );
int nGeoms = c.mGeometries.size();
mGeometries.resize( nGeoms );
Expand Down
98 changes: 69 additions & 29 deletions tests/src/core/geometry/testqgsgeometrycollection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "qgstest.h"
#include <QObject>
#include <QString>
#include <qtestcase.h>

#include "qgscircularstring.h"
#include "qgsgeometrycollection.h"
Expand All @@ -33,6 +34,8 @@ class TestQgsGeometryCollection: public QObject
Q_OBJECT
private slots:
void geometryCollection();
void testCopyConstructor();
void testAssignment();
};

void TestQgsGeometryCollection::geometryCollection()
Expand Down Expand Up @@ -196,36 +199,10 @@ void TestQgsGeometryCollection::geometryCollection()
QCOMPARE( *static_cast< const QgsLineString * >( cloned->geometryN( 0 ) ), part );
QCOMPARE( *static_cast< const QgsLineString * >( cloned->geometryN( 1 ) ), part2 );

//copy constructor
QgsGeometryCollection c12;
QgsGeometryCollection c13( c12 );
QVERIFY( c13.isEmpty() );
part.setPoints( QgsPointSequence() << QgsPoint( Qgis::WkbType::PointZM, 0, 0, 1, 5 )
<< QgsPoint( Qgis::WkbType::PointZM, 0, 10, 2, 6 ) << QgsPoint( Qgis::WkbType::PointZM, 10, 10, 3, 7 )
<< QgsPoint( Qgis::WkbType::PointZM, 10, 0, 4, 8 ) << QgsPoint( Qgis::WkbType::PointZM, 0, 0, 1, 9 ) );
c12.addGeometry( part.clone() );
part2.setPoints( QgsPointSequence() << QgsPoint( Qgis::WkbType::PointZM, 1, 1, 1, 2 )
<< QgsPoint( Qgis::WkbType::PointZM, 1, 9, 2, 3 ) << QgsPoint( Qgis::WkbType::PointZM, 9, 9, 3, 6 )
<< QgsPoint( Qgis::WkbType::PointZM, 9, 1, 4, 4 ) << QgsPoint( Qgis::WkbType::PointZM, 1, 1, 1, 7 ) );
c12.addGeometry( part2.clone() );
QgsGeometryCollection c14( c12 );
QCOMPARE( c14.numGeometries(), 2 );
QCOMPARE( *static_cast< const QgsLineString * >( c14.geometryN( 0 ) ), part );
QCOMPARE( *static_cast< const QgsLineString * >( c14.geometryN( 1 ) ), part2 );

//assignment operator
QgsGeometryCollection c15;
c15 = c13;
QCOMPARE( c15.numGeometries(), 0 );
c15 = c14;
QCOMPARE( c15.numGeometries(), 2 );
QCOMPARE( *static_cast< const QgsLineString * >( c15.geometryN( 0 ) ), part );
QCOMPARE( *static_cast< const QgsLineString * >( c15.geometryN( 1 ) ), part2 );

//equality
QgsGeometryCollection emptyCollection;
QVERIFY( !( emptyCollection == c15 ) );
QVERIFY( emptyCollection != c15 );
QVERIFY( !( emptyCollection == c11 ) );
QVERIFY( emptyCollection != c11 );
QgsPoint notCollection;
QVERIFY( !( emptyCollection == notCollection ) );
QVERIFY( emptyCollection != notCollection );
Expand All @@ -249,7 +226,7 @@ void TestQgsGeometryCollection::geometryCollection()
QVERIFY( ml2 == ml3 );

//toCurveType
std::unique_ptr< QgsGeometryCollection > curveType( c12.toCurveType() );
std::unique_ptr< QgsGeometryCollection > curveType( c11.toCurveType() );
QCOMPARE( curveType->wkbType(), Qgis::WkbType::GeometryCollection );
QCOMPARE( curveType->numGeometries(), 2 );
const QgsCompoundCurve *curve = static_cast< const QgsCompoundCurve * >( curveType->geometryN( 0 ) );
Expand Down Expand Up @@ -1505,6 +1482,69 @@ void TestQgsGeometryCollection::geometryCollection()
QVERIFY( b2.boundingBoxIntersects( polygon2.boundingBox3D() ) );
}

void TestQgsGeometryCollection::testCopyConstructor()
{
QgsGeometryCollection c1;
QgsLineString part;
QgsLineString part2;

part.setPoints( QgsPointSequence() << QgsPoint( 0, 0 ) << QgsPoint( 0, 10 ) << QgsPoint( 10, 10 )
<< QgsPoint( 10, 0 ) << QgsPoint( 0, 0 ) );
c1.addGeometry( part.clone() );

QgsGeometryCollection c2;
QgsGeometryCollection c3( c2 );
QVERIFY( c3.isEmpty() );
part.setPoints( QgsPointSequence() << QgsPoint( Qgis::WkbType::PointZM, 0, 0, 1, 5 )
<< QgsPoint( Qgis::WkbType::PointZM, 0, 10, 2, 6 ) << QgsPoint( Qgis::WkbType::PointZM, 10, 10, 3, 7 )
<< QgsPoint( Qgis::WkbType::PointZM, 10, 0, 4, 8 ) << QgsPoint( Qgis::WkbType::PointZM, 0, 0, 1, 9 ) );
c2.addGeometry( part.clone() );
part2.setPoints( QgsPointSequence() << QgsPoint( Qgis::WkbType::PointZM, 1, 1, 1, 2 )
<< QgsPoint( Qgis::WkbType::PointZM, 1, 9, 2, 3 ) << QgsPoint( Qgis::WkbType::PointZM, 9, 9, 3, 6 )
<< QgsPoint( Qgis::WkbType::PointZM, 9, 1, 4, 4 ) << QgsPoint( Qgis::WkbType::PointZM, 1, 1, 1, 7 ) );
c2.addGeometry( part2.clone() );
QgsGeometryCollection c4( c2 );
QCOMPARE( c4.numGeometries(), 2 );
QCOMPARE( *static_cast< const QgsLineString * >( c4.geometryN( 0 ) ), part );
QCOMPARE( *static_cast< const QgsLineString * >( c4.geometryN( 1 ) ), part2 );
}

void TestQgsGeometryCollection::testAssignment()
{
QgsGeometryCollection c1;
QgsLineString part;
QgsLineString part2;
QgsLineString part3;

part.setPoints( QgsPointSequence() << QgsPoint( 0, 0 ) << QgsPoint( 0, 10 ) << QgsPoint( 10, 10 )
<< QgsPoint( 10, 0 ) << QgsPoint( 0, 0 ) );
c1.addGeometry( part.clone() );
QCOMPARE( c1.numGeometries(), 1 );

QgsGeometryCollection c2;
QCOMPARE( c2.numGeometries(), 0 );
QVERIFY( c1 != c2 );

part2.setPoints( QgsPointSequence() << QgsPoint( Qgis::WkbType::PointZM, 1, 1, 1, 2 )
<< QgsPoint( Qgis::WkbType::PointZM, 1, 9, 2, 3 ) << QgsPoint( Qgis::WkbType::PointZM, 9, 9, 3, 6 )
<< QgsPoint( Qgis::WkbType::PointZM, 9, 1, 4, 4 ) << QgsPoint( Qgis::WkbType::PointZM, 1, 1, 1, 7 ) );
c2.addGeometry( part2.clone() );
QCOMPARE( c2.numGeometries(), 1 );
QVERIFY( c1 != c2 );

part3.setPoints( QgsPointSequence() << QgsPoint( 1, 1 )
<< QgsPoint( 1, 9 ) << QgsPoint( 9, 9 )
<< QgsPoint( 9, 1 ) << QgsPoint( 1, 1 ) );
c1.addGeometry( part3.clone() );

c2 = c1;
QCOMPARE( c1.numGeometries(), 2 );
QCOMPARE( c2.numGeometries(), 2 );
QCOMPARE( *static_cast< const QgsLineString * >( c2.geometryN( 0 ) ), part );
QCOMPARE( *static_cast< const QgsLineString * >( c2.geometryN( 1 ) ), part3 );
QVERIFY( c1 == c2 );
}


QGSTEST_MAIN( TestQgsGeometryCollection )
#include "testqgsgeometrycollection.moc"

0 comments on commit 9fed0f1

Please sign in to comment.