Skip to content
Permalink
Browse files

Avoid container detachments by using const methods wherever possible

eg QList::at() instead of QList:[], constFind instead of find, ...
  • Loading branch information
nyalldawson committed Dec 16, 2015
1 parent 6bfde51 commit 18614e11e3017016137daadf82fed6941b93a339
Showing with 680 additions and 768 deletions.
  1. +4 −6 src/analysis/interpolation/qgsidwinterpolator.cpp
  2. +8 −10 src/analysis/interpolation/qgsinterpolator.cpp
  3. +9 −11 src/analysis/interpolation/qgstininterpolator.cpp
  4. +2 −2 src/analysis/vector/qgszonalstatistics.cpp
  5. +13 −17 src/app/composer/qgscomposer.cpp
  6. +1 −1 src/app/composer/qgscomposerlegendlayersdialog.cpp
  7. +11 −11 src/app/composer/qgscomposermanager.cpp
  8. +2 −2 src/app/composer/qgscompositionwidget.cpp
  9. +1 −1 src/app/gps/qgsgpsinformationwidget.cpp
  10. +7 −7 src/app/nodetool/qgsselectedfeature.cpp
  11. +16 −16 src/app/pluginmanager/qgspluginmanager.cpp
  12. +2 −2 src/app/qgisapp.cpp
  13. +1 −1 src/app/qgsclipboard.cpp
  14. +2 −2 src/app/qgsfieldcalculator.cpp
  15. +2 −2 src/app/qgsfieldsproperties.cpp
  16. +1 −1 src/app/qgshandlebadlayers.cpp
  17. +1 −1 src/app/qgslabelpropertydialog.cpp
  18. +1 −1 src/app/qgsmaptoollabel.cpp
  19. +1 −1 src/app/qgsmaptoolsimplify.cpp
  20. +2 −2 src/app/qgsmergeattributesdialog.cpp
  21. +2 −2 src/app/qgspluginmetadata.cpp
  22. +2 −2 src/app/qgspluginmetadata.h
  23. +4 −4 src/app/qgspluginregistry.cpp
  24. +7 −7 src/app/qgsprojectproperties.cpp
  25. +2 −2 src/app/qgsshortcutsmanager.cpp
  26. +2 −2 src/core/composer/qgsatlascomposition.cpp
  27. +8 −10 src/core/composer/qgscomposerattributetable.cpp
  28. +3 −4 src/core/composer/qgscomposerattributetablemodel.cpp
  29. +3 −4 src/core/composer/qgscomposerattributetablemodelv2.cpp
  30. +16 −20 src/core/composer/qgscomposeritemgroup.cpp
  31. +1 −1 src/core/composer/qgscomposermapgrid.cpp
  32. +1 −1 src/core/composer/qgscomposermapoverview.cpp
  33. +7 −8 src/core/composer/qgscomposermodel.cpp
  34. +10 −13 src/core/composer/qgscomposermultiframe.cpp
  35. +2 −2 src/core/composer/qgscomposerobject.cpp
  36. +2 −2 src/core/composer/qgscomposertable.cpp
  37. +11 −25 src/core/composer/qgscomposition.cpp
  38. +6 −6 src/core/dxf/qgsdxfexport.cpp
  39. +4 −4 src/core/effects/qgseffectstack.cpp
  40. +7 −7 src/core/geometry/qgscircularstringv2.cpp
  41. +8 −10 src/core/geometry/qgscompoundcurvev2.cpp
  42. +8 −10 src/core/geometry/qgscurvepolygonv2.cpp
  43. +8 −10 src/core/geometry/qgsgeometrycollectionv2.cpp
  44. +1 −5 src/core/gps/qgsgpsconnectionregistry.cpp
  45. +4 −4 src/core/gps/qgsgpsdetector.cpp
  46. +3 −3 src/core/layertree/qgslayertreegroup.cpp
  47. +2 −2 src/core/pal/costcalculator.cpp
  48. +1 −1 src/core/pal/pal.cpp
  49. +1 −1 src/core/qgscrscache.cpp
  50. +9 −9 src/core/qgsdataitem.cpp
  51. +3 −3 src/core/qgsexpression.cpp
  52. +3 −3 src/core/qgsexpressioncontext.cpp
  53. +1 −1 src/core/qgsfield.cpp
  54. +1 −1 src/core/qgsgml.cpp
  55. +4 −4 src/core/qgslegendrenderer.cpp
  56. +3 −6 src/core/qgsmaprenderer.cpp
  57. +3 −8 src/core/qgspallabeling.cpp
  58. +1 −1 src/core/qgsproject.cpp
  59. +1 −2 src/core/qgsrulebasedlabeling.cpp
  60. +1 −1 src/core/qgsvectorlayer.cpp
  61. +3 −3 src/core/qgsvectorlayereditbuffer.cpp
  62. +11 −11 src/core/qgsvectorlayerundocommand.cpp
  63. +9 −9 src/core/raster/qgsrasterpipe.cpp
  64. +23 −24 src/core/symbology-ng/qgscategorizedsymbolrendererv2.cpp
  65. +4 −6 src/core/symbology-ng/qgscptcityarchive.cpp
  66. +1 −1 src/core/symbology-ng/qgsgeometrygeneratorsymbollayerv2.cpp
  67. +1 −1 src/core/symbology-ng/qgsgeometrygeneratorsymbollayerv2.h
  68. +29 −29 src/core/symbology-ng/qgsgraduatedsymbolrendererv2.cpp
  69. +1 −1 src/core/symbology-ng/qgsheatmaprenderer.cpp
  70. +6 −6 src/core/symbology-ng/qgsinvertedpolygonrenderer.cpp
  71. +4 −2 src/core/symbology-ng/qgspointdisplacementrenderer.cpp
  72. +18 −29 src/core/symbology-ng/qgsrulebasedrendererv2.cpp
  73. +5 −5 src/core/symbology-ng/qgsrulebasedrendererv2.h
  74. +3 −5 src/core/symbology-ng/qgsstylev2.cpp
  75. +1 −5 src/core/symbology-ng/qgssvgcache.cpp
  76. +6 −10 src/core/symbology-ng/qgssymbollayerv2.cpp
  77. +1 −1 src/core/symbology-ng/qgssymbollayerv2.h
  78. +2 −2 src/core/symbology-ng/qgssymbollayerv2registry.cpp
  79. +8 −10 src/core/symbology-ng/qgssymbolv2.cpp
  80. +7 −7 src/core/symbology-ng/qgsvectorcolorrampv2.cpp
  81. +3 −3 src/gui/attributetable/qgsattributetablemodel.cpp
  82. +4 −4 src/gui/auth/qgsauthconfigselect.cpp
  83. +1 −1 src/gui/editorwidgets/qgsrelationreferencewidget.cpp
  84. +16 −16 src/gui/qgsadvanceddigitizingdockwidget.cpp
  85. +2 −2 src/gui/qgscodeeditorpython.cpp
  86. +2 −2 src/gui/qgscolorswatchgrid.cpp
  87. +2 −2 src/gui/qgscomposerruler.cpp
  88. +1 −1 src/gui/qgsmessagebar.cpp
  89. +2 −2 src/gui/qgsowssourceselect.cpp
  90. +2 −2 src/gui/qgsrasterformatsaveoptionswidget.cpp
  91. +4 −4 src/gui/qgsrubberband.cpp
  92. +2 −2 src/gui/qgsuserinputdockwidget.cpp
  93. +2 −2 src/gui/symbology-ng/qgscptcitycolorrampv2dialog.cpp
  94. +5 −5 src/gui/symbology-ng/qgssymbollevelsv2dialog.cpp
  95. +37 −37 src/plugins/dxf2shp_converter/builder.cpp
  96. +4 −4 src/plugins/geometry_checker/ui/qgsgeometrycheckfixdialog.cpp
  97. +1 −1 src/plugins/georeferencer/qgsgeorefplugingui.cpp
  98. +2 −2 src/plugins/spatialquery/qgsspatialquerydialog.cpp
  99. +8 −10 src/plugins/topology/checkDock.cpp
  100. +1 −1 src/plugins/topology/rulesDialog.cpp
  101. +7 −7 src/plugins/topology/topolError.cpp
  102. +18 −25 src/plugins/topology/topolTest.cpp
  103. +3 −3 src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp
  104. +4 −4 src/providers/gdal/qgsgdalprovider.cpp
  105. +3 −3 src/providers/gpx/gpsdata.cpp
  106. +3 −3 src/providers/gpx/qgsgpxfeatureiterator.cpp
  107. +9 −9 src/providers/gpx/qgsgpxprovider.cpp
  108. +2 −2 src/providers/memory/qgsmemoryfeatureiterator.cpp
  109. +2 −2 src/providers/memory/qgsmemoryprovider.cpp
  110. +1 −1 src/providers/mssql/qgsmssqldataitems.cpp
  111. +2 −2 src/providers/ogr/qgsogrconnpool.h
  112. +2 −2 src/providers/postgres/qgspostgresconn.cpp
  113. +2 −2 src/providers/postgres/qgspostgresdataitems.cpp
  114. +1 −1 src/providers/postgres/qgspostgresfeatureiterator.cpp
  115. +8 −8 src/providers/postgres/qgspostgresprovider.cpp
  116. +6 −6 src/providers/wcs/qgswcsprovider.cpp
  117. +8 −8 src/providers/wfs/qgswfsprovider.cpp
  118. +6 −6 src/providers/wms/qgstilescalewidget.cpp
  119. +3 −3 src/providers/wms/qgswmscapabilities.cpp
  120. +1 −1 src/providers/wms/qgswmsdataitems.cpp
  121. +17 −17 src/providers/wms/qgswmsprovider.cpp
  122. +10 −10 src/providers/wms/qgswmssourceselect.cpp
  123. +2 −2 src/server/qgsmslayercache.cpp
  124. +10 −10 src/server/qgswcsserver.cpp
  125. +20 −20 src/server/qgswfsserver.cpp
  126. +27 −27 src/server/qgswmsserver.cpp
  127. +4 −4 tests/bench/qgsbench.cpp
  128. +2 −2 tests/src/core/testqgscoordinatereferencesystem.cpp
@@ -47,18 +47,16 @@ int QgsIDWInterpolator::interpolatePoint( double x, double y, double& result )
double sumCounter = 0;
double sumDenominator = 0;

QVector<vertexData>::iterator vertex_it = mCachedBaseData.begin();

for ( ; vertex_it != mCachedBaseData.end(); ++vertex_it )
Q_FOREACH ( const vertexData& vertex_it, mCachedBaseData )
{
distance = sqrt(( vertex_it->x - x ) * ( vertex_it->x - x ) + ( vertex_it->y - y ) * ( vertex_it->y - y ) );
distance = sqrt(( vertex_it.x - x ) * ( vertex_it.x - x ) + ( vertex_it.y - y ) * ( vertex_it.y - y ) );
if (( distance - 0 ) < std::numeric_limits<double>::min() )
{
result = vertex_it->z;
result = vertex_it.z;
return 0;
}
currentWeight = 1 / ( pow( distance, mDistanceCoefficient ) );
sumCounter += ( currentWeight * vertex_it->z );
sumCounter += ( currentWeight * vertex_it.z );
sumDenominator += currentWeight;
}

@@ -50,25 +50,23 @@ int QgsInterpolator::cacheBaseData()
mCachedBaseData.clear();
mCachedBaseData.reserve( 100000 );

QList<LayerData>::iterator v_it = mLayerData.begin();

for ( ; v_it != mLayerData.end(); ++v_it )
Q_FOREACH ( const LayerData& layer, mLayerData )
{
if ( v_it->vectorLayer == nullptr )
if ( !layer.vectorLayer )
{
continue;
}

QgsVectorLayer* vlayer = v_it->vectorLayer;
QgsVectorLayer* vlayer = layer.vectorLayer;
if ( !vlayer )
{
return 2;
}

QgsAttributeList attList;
if ( !v_it->zCoordInterpolation )
if ( !layer.zCoordInterpolation )
{
attList.push_back( v_it->interpolationAttribute );
attList.push_back( layer.interpolationAttribute );
}


@@ -80,9 +78,9 @@ int QgsInterpolator::cacheBaseData()
QgsFeature theFeature;
while ( fit.nextFeature( theFeature ) )
{
if ( !v_it->zCoordInterpolation )
if ( !layer.zCoordInterpolation )
{
QVariant attributeVariant = theFeature.attribute( v_it->interpolationAttribute );
QVariant attributeVariant = theFeature.attribute( layer.interpolationAttribute );
if ( !attributeVariant.isValid() ) //attribute not found, something must be wrong (e.g. NULL value)
{
continue;
@@ -94,7 +92,7 @@ int QgsInterpolator::cacheBaseData()
}
}

if ( addVerticesToCache( theFeature.constGeometry(), v_it->zCoordInterpolation, attributeValue ) != 0 )
if ( addVerticesToCache( theFeature.constGeometry(), layer.zCoordInterpolation, attributeValue ) != 0 )
{
return 3;
}
@@ -84,12 +84,11 @@ void QgsTINInterpolator::initialize()
int nProcessedFeatures = 0;
if ( mShowProgressDialog )
{
QList<LayerData>::iterator layerDataIt = mLayerData.begin();
for ( ; layerDataIt != mLayerData.end(); ++layerDataIt )
Q_FOREACH ( const LayerData& layer, mLayerData )
{
if ( layerDataIt->vectorLayer )
if ( layer.vectorLayer )
{
nFeatures += layerDataIt->vectorLayer->featureCount();
nFeatures += layer.vectorLayer->featureCount();
}
}
}
@@ -103,18 +102,17 @@ void QgsTINInterpolator::initialize()


QgsFeature f;
QList<LayerData>::iterator layerDataIt = mLayerData.begin();
for ( ; layerDataIt != mLayerData.end(); ++layerDataIt )
Q_FOREACH ( const LayerData& layer, mLayerData )
{
if ( layerDataIt->vectorLayer )
if ( layer.vectorLayer )
{
QgsAttributeList attList;
if ( !layerDataIt->zCoordInterpolation )
if ( !layer.zCoordInterpolation )
{
attList.push_back( layerDataIt->interpolationAttribute );
attList.push_back( layer.interpolationAttribute );
}

QgsFeatureIterator fit = layerDataIt->vectorLayer->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( attList ) );
QgsFeatureIterator fit = layer.vectorLayer->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( attList ) );

while ( fit.nextFeature( f ) )
{
@@ -126,7 +124,7 @@ void QgsTINInterpolator::initialize()
}
theProgressDialog->setValue( nProcessedFeatures );
}
insertData( &f, layerDataIt->zCoordInterpolation, layerDataIt->interpolationAttribute, layerDataIt->mInputType );
insertData( &f, layer.zCoordInterpolation, layer.interpolationAttribute, layer.mInputType );
++nProcessedFeatures;
}
}
@@ -319,11 +319,11 @@ int QgsZonalStatistics::calculateStatistics( QProgressDialog* p )
double medianValue;
if ( even )
{
medianValue = ( featureStats.values[size / 2 - 1] + featureStats.values[size / 2] ) / 2;
medianValue = ( featureStats.values.at( size / 2 - 1 ) + featureStats.values.at( size / 2 ) ) / 2;
}
else //odd
{
medianValue = featureStats.values[( size + 1 ) / 2 - 1];
medianValue = featureStats.values.at(( size + 1 ) / 2 - 1 );
}
changeAttributeMap.insert( medianIndex, QVariant( medianValue ) );
}
@@ -912,7 +912,7 @@ void QgsComposer::updateStatusZoom()

void QgsComposer::statusZoomCombo_currentIndexChanged( int index )
{
double selectedZoom = mStatusZoomLevelsList[ mStatusZoomLevelsList.count() - index - 1 ];
double selectedZoom = mStatusZoomLevelsList.at( mStatusZoomLevelsList.count() - index - 1 );
if ( mView )
{
mView->setZoomLevel( selectedZoom );
@@ -955,7 +955,7 @@ void QgsComposer::showItemOptions( QgsComposerItem* item )
return;
}

QMap<QgsComposerItem*, QWidget*>::iterator it = mItemWidgetMap.find( item );
QMap<QgsComposerItem*, QWidget*>::const_iterator it = mItemWidgetMap.constFind( item );
if ( it == mItemWidgetMap.constEnd() )
{
return;
@@ -2383,13 +2383,13 @@ struct QgsItemTempHider
}
void hideAll()
{
QgsItemVisibilityHash::iterator it = mItemVisibility.begin();
for ( ; it != mItemVisibility.end(); ++it ) it.key()->hide();
QgsItemVisibilityHash::const_iterator it = mItemVisibility.constBegin();
for ( ; it != mItemVisibility.constEnd(); ++it ) it.key()->hide();
}
~QgsItemTempHider()
{
QgsItemVisibilityHash::iterator it = mItemVisibility.begin();
for ( ; it != mItemVisibility.end(); ++it )
QgsItemVisibilityHash::const_iterator it = mItemVisibility.constBegin();
for ( ; it != mItemVisibility.constEnd(); ++it )
{
it.key()->setVisible( it.value() );
}
@@ -3426,8 +3426,8 @@ void QgsComposer::writeXML( QDomNode& parentNode, QDomDocument& doc )
composerElem.setAttribute( "title", mTitle );

//change preview mode of minimised / hidden maps before saving XML (show contents only on demand)
QMap< QgsComposerMap*, int >::iterator mapIt = mMapsToRestore.begin();
for ( ; mapIt != mMapsToRestore.end(); ++mapIt )
QMap< QgsComposerMap*, int >::const_iterator mapIt = mMapsToRestore.constBegin();
for ( ; mapIt != mMapsToRestore.constEnd(); ++mapIt )
{
mapIt.key()->setPreviewMode(( QgsComposerMap::PreviewMode )( mapIt.value() ) );
}
@@ -3629,11 +3629,7 @@ void QgsComposer::restoreGridSettings()
void QgsComposer::deleteItemWidgets()
{
//delete all the items
QMap<QgsComposerItem*, QWidget*>::iterator it = mItemWidgetMap.begin();
for ( ; it != mItemWidgetMap.end(); ++it )
{
delete it.value();
}
qDeleteAll( mItemWidgetMap );
mItemWidgetMap.clear();
}

@@ -3749,9 +3745,9 @@ void QgsComposer::addComposerHtmlFrame( QgsComposerHtml* html, QgsComposerFrame*

void QgsComposer::deleteItem( QgsComposerItem* item )
{
QMap<QgsComposerItem*, QWidget*>::iterator it = mItemWidgetMap.find( item );
QMap<QgsComposerItem*, QWidget*>::const_iterator it = mItemWidgetMap.constFind( item );

if ( it == mItemWidgetMap.end() )
if ( it == mItemWidgetMap.constEnd() )
{
return;
}
@@ -3937,8 +3933,8 @@ void QgsComposer::restoreComposerMapStates()
if ( !mMapsToRestore.isEmpty() )
{
//go through maps and restore original preview modes (show on demand after loading from project file)
QMap< QgsComposerMap*, int >::iterator mapIt = mMapsToRestore.begin();
for ( ; mapIt != mMapsToRestore.end(); ++mapIt )
QMap< QgsComposerMap*, int >::const_iterator mapIt = mMapsToRestore.constBegin();
for ( ; mapIt != mMapsToRestore.constEnd(); ++mapIt )
{
mapIt.key()->setPreviewMode(( QgsComposerMap::PreviewMode )( mapIt.value() ) );
mapIt.key()->cache();
@@ -46,7 +46,7 @@ QgsMapLayer* QgsComposerLegendLayersDialog::selectedLayer()
return nullptr;
}

QMap<QListWidgetItem*, QgsMapLayer*>::iterator it = mItemLayerMap.find( item );
QMap<QListWidgetItem*, QgsMapLayer*>::const_iterator it = mItemLayerMap.constFind( item );
QgsMapLayer* c = nullptr;
c = it.value();
return c;
@@ -94,8 +94,8 @@ void QgsComposerManager::refreshComposers()
QSet<QgsComposer *> selectedComposers;
Q_FOREACH ( QListWidgetItem* item, mComposerListWidget->selectedItems() )
{
QMap<QListWidgetItem*, QgsComposer*>::iterator it = mItemComposerMap.find( item );
if ( it != mItemComposerMap.end() )
QMap<QListWidgetItem*, QgsComposer*>::const_iterator it = mItemComposerMap.constFind( item );
if ( it != mItemComposerMap.constEnd() )
{
selectedComposers << it.value();
}
@@ -414,8 +414,8 @@ void QgsComposerManager::remove_clicked()
// Find the QgsComposers that need to be deleted
Q_FOREACH ( QListWidgetItem* item, composerItems )
{
QMap<QListWidgetItem*, QgsComposer*>::iterator it = mItemComposerMap.find( item );
if ( it != mItemComposerMap.end() )
QMap<QListWidgetItem*, QgsComposer*>::const_iterator it = mItemComposerMap.constFind( item );
if ( it != mItemComposerMap.constEnd() )
{
composerList << it.value();
}
@@ -432,7 +432,7 @@ void QgsComposerManager::show_clicked()
{
Q_FOREACH ( QListWidgetItem* item, mComposerListWidget->selectedItems() )
{
QMap<QListWidgetItem*, QgsComposer*>::const_iterator it = mItemComposerMap.find( item );
QMap<QListWidgetItem*, QgsComposer*>::const_iterator it = mItemComposerMap.constFind( item );
if ( it != mItemComposerMap.constEnd() )
{
QgsComposer* c = nullptr;
@@ -468,8 +468,8 @@ void QgsComposerManager::duplicate_clicked()
QString currentTitle;

QListWidgetItem* item = mComposerListWidget->selectedItems().at( 0 );
QMap<QListWidgetItem*, QgsComposer*>::iterator it = mItemComposerMap.find( item );
if ( it != mItemComposerMap.end() )
QMap<QListWidgetItem*, QgsComposer*>::const_iterator it = mItemComposerMap.constFind( item );
if ( it != mItemComposerMap.constEnd() )
{
currentComposer = it.value();
currentTitle = it.value()->title();
@@ -519,8 +519,8 @@ void QgsComposerManager::rename_clicked()
QgsComposer* currentComposer = nullptr;

QListWidgetItem* item = mComposerListWidget->selectedItems().at( 0 );
QMap<QListWidgetItem*, QgsComposer*>::iterator it = mItemComposerMap.find( item );
if ( it != mItemComposerMap.end() )
QMap<QListWidgetItem*, QgsComposer*>::const_iterator it = mItemComposerMap.constFind( item );
if ( it != mItemComposerMap.constEnd() )
{
currentComposer = it.value();
currentTitle = it.value()->title();
@@ -543,8 +543,8 @@ void QgsComposerManager::rename_clicked()

void QgsComposerManager::on_mComposerListWidget_itemChanged( QListWidgetItem * item )
{
QMap<QListWidgetItem*, QgsComposer*>::iterator it = mItemComposerMap.find( item );
if ( it != mItemComposerMap.end() )
QMap<QListWidgetItem*, QgsComposer*>::const_iterator it = mItemComposerMap.constFind( item );
if ( it != mItemComposerMap.constEnd() )
{
it.value()->setTitle( item->text() );
}
@@ -460,8 +460,8 @@ void QgsCompositionWidget::applyCurrentPaperSettings()
if ( mComposition )
{
//find entry in mPaper map to set width and height
QMap<QString, QgsCompositionPaper>::iterator it = mPaperMap.find( mPaperSizeComboBox->currentText() );
if ( it == mPaperMap.end() )
QMap<QString, QgsCompositionPaper>::const_iterator it = mPaperMap.constFind( mPaperSizeComboBox->currentText() );
if ( it == mPaperMap.constEnd() )
{
return;
}
@@ -880,7 +880,7 @@ void QgsGPSInformationWidget::on_mBtnCloseFeature_clicked()
memcpy( &wkb[1+sizeof( int )], &length, sizeof( int ) );
int position = 1 + 2 * sizeof( int );
double x, y;
for ( QList<QgsPoint>::iterator it = mCaptureList.begin(); it != mCaptureList.end(); ++it )
for ( QList<QgsPoint>::const_iterator it = mCaptureList.constBegin(); it != mCaptureList.constEnd(); ++it )
{
QgsPoint savePoint = *it;
// transform the gps point into the layer crs
@@ -249,13 +249,13 @@ void QgsSelectedFeature::deleteSelectedVertexes()
int count = 0;
for ( int i = mVertexMap.size() - 1; i > -1 && nSelected > 0; i-- )
{
if ( mVertexMap[i]->isSelected() )
if ( mVertexMap.at( i )->isSelected() )
{
if ( topologicalEditing )
{
// snap from current vertex
currentResultList.clear();
mVlayer->snapWithContext( mVertexMap[i]->pointV1(), ZERO_TOLERANCE, currentResultList, QgsSnapper::SnapToVertex );
mVlayer->snapWithContext( mVertexMap.at( i )->pointV1(), ZERO_TOLERANCE, currentResultList, QgsSnapper::SnapToVertex );
}

// only last update should trigger the geometry update
@@ -385,7 +385,7 @@ void QgsSelectedFeature::deleteVertexMap()

bool QgsSelectedFeature::isSelected( int vertexNr )
{
return mVertexMap[vertexNr]->isSelected();
return mVertexMap.at( vertexNr )->isSelected();
}

QgsGeometry *QgsSelectedFeature::geometry()
@@ -427,7 +427,7 @@ void QgsSelectedFeature::selectVertex( int vertexNr )
if ( vertexNr < 0 || vertexNr >= mVertexMap.size() )
return;

QgsVertexEntry *entry = mVertexMap[vertexNr];
QgsVertexEntry *entry = mVertexMap.at( vertexNr );
entry->setSelected();

emit selectionChanged();
@@ -438,7 +438,7 @@ void QgsSelectedFeature::deselectVertex( int vertexNr )
if ( vertexNr < 0 || vertexNr >= mVertexMap.size() )

This comment has been minimized.

Copy link
@m-kuhn

m-kuhn Dec 16, 2015

Member

Wouldn't it be nicer to use value() For places where a check previous to .at() is done with a subsequent if( entry ) return;?
It's easier to read and we're sure not to forget the i < 0 part.
Mainly asking for your opinion, not asking you to change it

This comment has been minimized.

Copy link
@nyalldawson

nyalldawson Dec 16, 2015

Author Collaborator

I actually don't like value() much. I lean toward the explicit checks as it makes it obvious what the behavior is for out of range indexes, and forces me to think about what the logical outcome should be in this scenario. It also makes it easier to debug, as the crash will occur right where the out of bounds access happens rather than somewhere else crashing because a null pointer is de-referenced. I also don't find .value()'s behaviour to be very "readable", as it doesn't match this standard c++ behaviour and may trip up devs who are new to Qt.

This comment has been minimized.

Copy link
@m-kuhn

m-kuhn Dec 16, 2015

Member

It also makes it easier to debug, as the crash will occur right where the out of bounds access happens...

... if run on a developer's computer, if not a user will loose data since his last save. But admitted that also happens if a nullptr is dereferenced.

I like .value() because it's much faster to write than the check and I'm lazy. It also avoids off-by-one errors (i.e. writing i > size() instead of i >= size())

I recently found this:
b319435#diff-b3a61e32c4813473839a855b5cd6fd8aL85

Apparently nobody has called it with n<0 before but if we assume this we can as well assume the upper bound to be guaranteed.

And I don't like duplicated code ;)

return;

QgsVertexEntry *entry = mVertexMap[vertexNr];
QgsVertexEntry *entry = mVertexMap.at( vertexNr );
entry->setSelected( false );

emit selectionChanged();
@@ -448,7 +448,7 @@ void QgsSelectedFeature::deselectAllVertexes()
{
for ( int i = 0; i < mVertexMap.size(); i++ )
{
mVertexMap[i]->setSelected( false );
mVertexMap.at( i )->setSelected( false );
}
emit selectionChanged();
}
@@ -458,7 +458,7 @@ void QgsSelectedFeature::invertVertexSelection( int vertexNr )
if ( vertexNr < 0 || vertexNr >= mVertexMap.size() )
return;

QgsVertexEntry *entry = mVertexMap[vertexNr];
QgsVertexEntry *entry = mVertexMap.at( vertexNr );

bool selected = !entry->isSelected();

1 comment on commit 18614e1

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn commented on 18614e1 Dec 16, 2015

Thanks a lot!

Please sign in to comment.
You can’t perform that action at this time.