Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Also avoid detaching QgsFields and QgsAttributes where possible
  • Loading branch information
nyalldawson committed Oct 20, 2015
1 parent 9a94132 commit 1969e09
Show file tree
Hide file tree
Showing 62 changed files with 206 additions and 206 deletions.
4 changes: 2 additions & 2 deletions src/analysis/vector/qgsgeometryanalyzer.cpp
Expand Up @@ -485,8 +485,8 @@ bool QgsGeometryAnalyzer::convexHull( QgsVectorLayer* layer, const QString& shap
values = simpleMeasure( dissolveGeometry );
QgsAttributes attributes( 3 );
attributes[0] = QVariant( currentKey );
attributes[1] = values[ 0 ];
attributes[2] = values[ 1 ];
attributes[1] = values.at( 0 );
attributes[2] = values.at( 1 );
QgsFeature dissolveFeature;
dissolveFeature.setAttributes( attributes );
dissolveFeature.setGeometry( dissolveGeometry );
Expand Down
2 changes: 1 addition & 1 deletion src/analysis/vector/qgsoverlayanalyzer.cpp
Expand Up @@ -185,7 +185,7 @@ void QgsOverlayAnalyzer::combineFieldLists( QgsFields& fieldListA, const QgsFiel
{
QList<QString> names;
for ( int idx = 0; idx < fieldListA.count(); ++idx )
names.append( fieldListA[idx].name() );
names.append( fieldListA.at( idx ).name() );

for ( int idx = 0; idx < fieldListB.count(); ++idx )
{
Expand Down
12 changes: 6 additions & 6 deletions src/app/qgisapp.cpp
Expand Up @@ -6164,14 +6164,14 @@ void QgisApp::mergeAttributesOfSelectedFeatures()

QVariant val = merged.at( i );
// convert to destination data type
if ( ! vl->fields()[i].convertCompatible( val ) )
if ( ! vl->fields().at( i ).convertCompatible( val ) )
{
if ( firstFeature )
{
//only warn on first feature
messageBar()->pushMessage(
tr( "Invalid result" ),
tr( "Could not store value '%1' in field of type %2" ).arg( merged.at( i ).toString(), vl->fields()[i].typeName() ),
tr( "Could not store value '%1' in field of type %2" ).arg( merged.at( i ).toString(), vl->fields().at( i ).typeName() ),
QgsMessageBar::WARNING );
}
}
Expand Down Expand Up @@ -6301,11 +6301,11 @@ void QgisApp::mergeSelectedFeatures()
{
QVariant val = attrs.at( i );
// convert to destination data type
if ( ! vl->fields()[i].convertCompatible( val ) )
if ( ! vl->fields().at( i ).convertCompatible( val ) )
{
messageBar()->pushMessage(
tr( "Invalid result" ),
tr( "Could not store value '%1' in field of type %2" ).arg( attrs.at( i ).toString(), vl->fields()[i].typeName() ),
tr( "Could not store value '%1' in field of type %2" ).arg( attrs.at( i ).toString(), vl->fields().at( i ).typeName() ),
QgsMessageBar::WARNING );
}
attrs[i] = val;
Expand Down Expand Up @@ -6527,13 +6527,13 @@ void QgisApp::editPaste( QgsMapLayer *destinationLayer )
if ( pkAttrList.contains( dst ) )
{
dstAttr[ dst ] = pasteVectorLayer->dataProvider()->defaultValue( dst );
if ( !dstAttr[ dst ].isNull() )
if ( !dstAttr.at( dst ).isNull() )
continue;
else if ( pasteVectorLayer->providerType() == "spatialite" )
continue;
}

dstAttr[ dst ] = srcAttr[ src ];
dstAttr[ dst ] = srcAttr.at( src );
}

featureIt->setAttributes( dstAttr );
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgsattributetabledialog.cpp
Expand Up @@ -389,7 +389,7 @@ void QgsAttributeTableDialog::runFieldCalculation( QgsVectorLayer* layer, const
<< QgsExpressionContextUtils::projectScope()
<< QgsExpressionContextUtils::layerScope( layer );

QgsField fld = layer->fields()[ fieldindex ];
QgsField fld = layer->fields().at( fieldindex );

//go through all the features and change the new attributes
QgsFeatureIterator fit = layer->getFeatures( request );
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgsattributetypedialog.cpp
Expand Up @@ -43,7 +43,7 @@ QgsAttributeTypeDialog::QgsAttributeTypeDialog( QgsVectorLayer *vl, int fieldIdx
, mFieldIdx( fieldIdx )
{
setupUi( this );
setWindowTitle( tr( "Edit Widget Properties - %1 (%2)" ).arg( vl->fields()[fieldIdx].name(), vl->name() ) );
setWindowTitle( tr( "Edit Widget Properties - %1 (%2)" ).arg( vl->fields().at( fieldIdx ).name(), vl->name() ) );

connect( selectionListWidget, SIGNAL( currentRowChanged( int ) ), this, SLOT( setStackPage( int ) ) );

Expand Down
4 changes: 2 additions & 2 deletions src/app/qgsclipboard.cpp
Expand Up @@ -91,7 +91,7 @@ void QgsClipboard::setSystemClipboard()

for ( int idx = 0; idx < mFeatureFields.count(); ++idx )
{
textFields += mFeatureFields[idx].name();
textFields += mFeatureFields.at( idx ).name();
}
textLines += textFields.join( "\t" );
textFields.clear();
Expand All @@ -116,7 +116,7 @@ void QgsClipboard::setSystemClipboard()
for ( int idx = 0; idx < attributes.count(); ++idx )
{
// QgsDebugMsg(QString("inspecting field '%1'.").arg(it2->toString()));
textFields += attributes[idx].toString();
textFields += attributes.at( idx ).toString();
}

textLines += textFields.join( "\t" );
Expand Down
4 changes: 2 additions & 2 deletions src/app/qgsdxfexportdialog.cpp
Expand Up @@ -72,7 +72,7 @@ void FieldSelectorDelegate::setEditorData( QWidget *editor, const QModelIndex &i

int idx = m->attributeIndex( vl );
if ( vl->fields().exists( idx ) )
fcb->setField( vl->fields()[ idx ].name() );
fcb->setField( vl->fields().at( idx ).name() );
}

void FieldSelectorDelegate::setModelData( QWidget *editor, QAbstractItemModel *model, const QModelIndex &index ) const
Expand Down Expand Up @@ -217,7 +217,7 @@ QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex& idx, int role
if ( role == Qt::DisplayRole )
{
if ( vl->fields().exists( idx ) )
return vl->fields()[ idx ].name();
return vl->fields().at( idx ).name();
else
return vl->name();
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/qgsfeatureaction.cpp
Expand Up @@ -231,10 +231,10 @@ void QgsFeatureAction::onFeatureSaved( const QgsFeature& feature )
{
QgsAttributes newValues = feature.attributes();
QgsAttributeMap origValues = sLastUsedValues[ mLayer ];
if ( origValues[idx] != newValues[idx] )
if ( origValues[idx] != newValues.at( idx ) )
{
QgsDebugMsg( QString( "saving %1 for %2" ).arg( sLastUsedValues[ mLayer ][idx].toString() ).arg( idx ) );
sLastUsedValues[ mLayer ][idx] = newValues[idx];
sLastUsedValues[ mLayer ][idx] = newValues.at( idx );
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgsfieldcalculator.cpp
Expand Up @@ -252,7 +252,7 @@ void QgsFieldCalculator::accept()
bool useGeometry = exp.needsGeometry();
int rownum = 1;

QgsField field = mVectorLayer->fields()[mAttributeId];
QgsField field = mVectorLayer->fields().at( mAttributeId );

bool newField = !mUpdateExistingGroupBox->isChecked();
QVariant emptyAttribute;
Expand Down
12 changes: 6 additions & 6 deletions src/app/qgsidentifyresultsdialog.cpp
Expand Up @@ -471,7 +471,7 @@ void QgsIdentifyResultsDialog::addFeature( QgsVectorLayer *vlayer, const QgsFeat
if ( i >= fields.count() )
continue;

QString value = fields[i].displayString( attrs[i] );
QString value = fields.at( i ).displayString( attrs.at( i ) );
QTreeWidgetItem *attrItem = new QTreeWidgetItem( QStringList() << QString::number( i ) << value );

attrItem->setData( 0, Qt::DisplayRole, vlayer->attributeDisplayName( i ) );
Expand All @@ -486,7 +486,7 @@ void QgsIdentifyResultsDialog::addFeature( QgsVectorLayer *vlayer, const QgsFeat
continue;
}

value = representValue( vlayer, fields[i].name(), attrs[i] );
value = representValue( vlayer, fields[i].name(), attrs.at( i ) );

attrItem->setData( 1, Qt::DisplayRole, value );

Expand All @@ -513,8 +513,8 @@ void QgsIdentifyResultsDialog::addFeature( QgsVectorLayer *vlayer, const QgsFeat
if ( i >= fields.count() )
continue;

QString value = fields[i].displayString( attrs[i] );
QString value2 = representValue( vlayer, fields[i].name(), value );
QString value = fields.at( i ).displayString( attrs.at( i ) );
QString value2 = representValue( vlayer, fields.at( i ).name(), value );

tblResults->setRowCount( j + 1 );

Expand Down Expand Up @@ -734,11 +734,11 @@ void QgsIdentifyResultsDialog::addFeature( QgsRasterLayer *layer,
if ( i >= fields.count() )
continue;

QTreeWidgetItem *attrItem = new QTreeWidgetItem( QStringList() << QString::number( i ) << attrs[i].toString() );
QTreeWidgetItem *attrItem = new QTreeWidgetItem( QStringList() << QString::number( i ) << attrs.at( i ).toString() );

attrItem->setData( 0, Qt::DisplayRole, fields[i].name() );

QVariant value = attrs[i];
QVariant value = attrs.at( i );
attrItem->setData( 1, Qt::DisplayRole, value );
featItem->addChild( attrItem );
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/qgslabelpropertydialog.cpp
Expand Up @@ -96,7 +96,7 @@ void QgsLabelPropertyDialog::init( const QString& layerId, int featureId, const
mCurLabelField = vlayer->fieldNameIndex( labelFieldName );
if ( mCurLabelField >= 0 )
{
mLabelTextLineEdit->setText( attributeValues[mCurLabelField].toString() );
mLabelTextLineEdit->setText( attributeValues.at( mCurLabelField ).toString() );
const QgsFields& layerFields = vlayer->fields();
switch ( layerFields[mCurLabelField].type() )
{
Expand Down
8 changes: 4 additions & 4 deletions src/app/qgsmaptoollabel.cpp
Expand Up @@ -484,10 +484,10 @@ bool QgsMapToolLabel::dataDefinedPosition( QgsVectorLayer* vlayer, const QgsFeat
}

QgsAttributes attributes = f.attributes();
if ( !attributes[xCol].isNull() )
x = attributes[xCol].toDouble( &xSuccess );
if ( !attributes[yCol].isNull() )
y = attributes[yCol].toDouble( &ySuccess );
if ( !attributes.at( xCol ).isNull() )
x = attributes.at( xCol ).toDouble( &xSuccess );
if ( !attributes.at( yCol ).isNull() )
y = attributes.at( yCol ).toDouble( &ySuccess );

return true;
}
Expand Down
14 changes: 7 additions & 7 deletions src/app/qgsmergeattributesdialog.cpp
Expand Up @@ -127,10 +127,10 @@ void QgsMergeAttributesDialog::createTableWidgetContents()
{
int idx = mTableWidget->horizontalHeaderItem( j )->data( Qt::UserRole ).toInt();

QTableWidgetItem* attributeValItem = new QTableWidgetItem( attrs[idx].toString() );
QTableWidgetItem* attributeValItem = new QTableWidgetItem( attrs.at( idx ).toString() );
attributeValItem->setFlags( Qt::ItemIsEnabled | Qt::ItemIsSelectable );
mTableWidget->setItem( i + 1, j, attributeValItem );
mTableWidget->setCellWidget( i + 1, j, QgsAttributeEditor::createAttributeEditor( mTableWidget, NULL, mVectorLayer, idx, attrs[idx] ) );
mTableWidget->setCellWidget( i + 1, j, QgsAttributeEditor::createAttributeEditor( mTableWidget, NULL, mVectorLayer, idx, attrs.at( idx ) ) );
}
}

Expand Down Expand Up @@ -307,7 +307,7 @@ QVariant QgsMergeAttributesDialog::featureAttribute( int featureId, int col )
}
else
{
return QVariant( mVectorLayer->fields()[col].type() );
return QVariant( mVectorLayer->fields().at( col ).type() );
}
}

Expand All @@ -333,7 +333,7 @@ QVariant QgsMergeAttributesDialog::minimumAttribute( int col )

if ( numberOfConsideredFeatures < 1 )
{
return QVariant( mVectorLayer->fields()[col].type() );
return QVariant( mVectorLayer->fields().at( col ).type() );
}

return QVariant( minimumValue );
Expand Down Expand Up @@ -361,7 +361,7 @@ QVariant QgsMergeAttributesDialog::maximumAttribute( int col )

if ( numberOfConsideredFeatures < 1 )
{
return QVariant( mVectorLayer->fields()[col].type() );
return QVariant( mVectorLayer->fields().at( col ).type() );
}

return QVariant( maximumValue );
Expand All @@ -386,7 +386,7 @@ QVariant QgsMergeAttributesDialog::meanAttribute( int col )

if ( numberOfConsideredFeatures < 1 )
{
return QVariant( mVectorLayer->fields()[col].type() );
return QVariant( mVectorLayer->fields().at( col ).type() );
}

double mean = sum / numberOfConsideredFeatures;
Expand Down Expand Up @@ -417,7 +417,7 @@ QVariant QgsMergeAttributesDialog::medianAttribute( int col )

if ( size < 1 )
{
return QVariant( mVectorLayer->fields()[col].type() );
return QVariant( mVectorLayer->fields().at( col ).type() );
}

bool even = ( size % 2 ) < 1;
Expand Down
4 changes: 2 additions & 2 deletions src/core/diagram/qgshistogramdiagram.cpp
Expand Up @@ -93,11 +93,11 @@ QSizeF QgsHistogramDiagram::diagramSize( const QgsAttributes& attributes, const
return QSizeF(); //zero size if no attributes
}

double maxValue = attributes[0].toDouble();
double maxValue = attributes.at( 0 ).toDouble();

for ( int i = 0; i < attributes.count(); ++i )
{
maxValue = qMax( attributes[i].toDouble(), maxValue );
maxValue = qMax( attributes.at( i ).toDouble(), maxValue );
}

switch ( s.diagramOrientation )
Expand Down
4 changes: 2 additions & 2 deletions src/core/qgsexpression.cpp
Expand Up @@ -2637,7 +2637,7 @@ QString QgsExpression::replaceExpressionText( const QString &action, const QgsFe
QgsVectorLayer *layer,
const QMap<QString, QVariant> *substitutionMap, const QgsDistanceArea *distanceArea )
{
QgsExpressionContext context = QgsExpressionContextUtils::createFeatureBasedContext( feat ? *feat : QgsFeature(), layer ? layer->pendingFields() : QgsFields() );
QgsExpressionContext context = QgsExpressionContextUtils::createFeatureBasedContext( feat ? *feat : QgsFeature(), layer ? layer->fields() : QgsFields() );
return replaceExpressionText( action, &context, substitutionMap, distanceArea );
}

Expand Down Expand Up @@ -3353,7 +3353,7 @@ bool QgsExpression::NodeColumnRef::prepare( QgsExpression *parent, const QgsExpr

for ( int i = 0; i < fields.count(); ++i )
{
if ( QString::compare( fields[i].name(), mName, Qt::CaseInsensitive ) == 0 )
if ( QString::compare( fields.at( i ).name(), mName, Qt::CaseInsensitive ) == 0 )
{
mIndex = i;
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/core/qgsfeature.cpp
Expand Up @@ -231,7 +231,7 @@ QVariant QgsFeature::attribute( int fieldIdx ) const
if ( fieldIdx < 0 || fieldIdx >= d->attributes.count() )
return QVariant();

return d->attributes[fieldIdx];
return d->attributes.at( fieldIdx );
}


Expand All @@ -241,7 +241,7 @@ QVariant QgsFeature::attribute( const QString& name ) const
if ( fieldIdx == -1 )
return QVariant();

return d->attributes[fieldIdx];
return d->attributes.at( fieldIdx );
}

int QgsFeature::fieldNameIndex( const QString& fieldName ) const
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgslabel.cpp
Expand Up @@ -716,7 +716,7 @@ bool QgsLabel::readLabelField( QDomElement &el, int attr, const QString& prefix
int idx = 0;
for ( ; idx < mFields.count(); ++idx )
{
if ( mFields[idx].name() == name )
if ( mFields.at( idx ).name() == name )
{
break;
}
Expand Down
8 changes: 4 additions & 4 deletions src/core/qgsofflineediting.cpp
Expand Up @@ -622,7 +622,7 @@ QgsVectorLayer* QgsOfflineEditing::copyVectorLayer( QgsVectorLayer* layer, sqlit
QgsAttributes newAttrs( attrs.count() );
for ( int it = 0; it < attrs.count(); ++it )
{
newAttrs[column++] = attrs[it];
newAttrs[column++] = attrs.at( it );
}
f.setAttributes( newAttrs );

Expand Down Expand Up @@ -747,15 +747,15 @@ void QgsOfflineEditing::applyFeaturesAdded( QgsVectorLayer* offlineLayer, QgsVec
QgsAttributes attrs = f.attributes();
for ( int it = 0; it < attrs.count(); ++it )
{
newAttrs[ attrLookup[ it ] ] = attrs[ it ];
newAttrs[ attrLookup[ it ] ] = attrs.at( it );
}

// try to use default value from the provider
// (important especially e.g. for postgis primary key generated from a sequence)
for ( int k = 0; k < newAttrs.count(); ++k )
{
if ( newAttrs[k].isNull() && !defaultValues[k].isNull() )
newAttrs[k] = defaultValues[k];
if ( newAttrs.at( k ).isNull() && !defaultValues.at( k ).isNull() )
newAttrs[k] = defaultValues.at( k );
}

f.setAttributes( newAttrs );
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsprojectfiletransform.cpp
Expand Up @@ -355,7 +355,7 @@ void QgsProjectFileTransform::transform0110to1000()
int fieldNumber = classificationFieldElem.text().toInt();
if ( fieldNumber >= 0 && fieldNumber < theFields.count() )
{
QDomText fieldName = mDom.createTextNode( theFields[fieldNumber].name() );
QDomText fieldName = mDom.createTextNode( theFields.at( fieldNumber ).name() );
QDomNode nameNode = classificationFieldElem.firstChild();
classificationFieldElem.replaceChild( fieldName, nameNode );
}
Expand Down
4 changes: 2 additions & 2 deletions src/core/qgsrelation.cpp
Expand Up @@ -193,12 +193,12 @@ QgsFeatureRequest QgsRelation::getReferencedFeatureRequest( const QgsAttributes&
if ( referencedField.type() == QVariant::String )
{
// Use quotes
conditions << QString( "\"%1\" = '%2'" ).arg( fieldPair.referencedField(), attributes[referencingIdx].toString() );
conditions << QString( "\"%1\" = '%2'" ).arg( fieldPair.referencedField(), attributes.at( referencingIdx ).toString() );
}
else
{
// No quotes
conditions << QString( "\"%1\" = %2" ).arg( fieldPair.referencedField(), attributes[referencingIdx].toString() );
conditions << QString( "\"%1\" = %2" ).arg( fieldPair.referencedField(), attributes.at( referencingIdx ).toString() );
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsvectordataprovider.cpp
Expand Up @@ -410,7 +410,7 @@ void QgsVectorDataProvider::fillMinMaxCache()
QgsAttributes attrs = f.attributes();
for ( QgsAttributeList::const_iterator it = keys.begin(); it != keys.end(); ++it )
{
const QVariant& varValue = attrs[*it];
const QVariant& varValue = attrs.at( *it );

if ( varValue.isNull() )
continue;
Expand Down

6 comments on commit 1969e09

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 1969e09 Oct 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably already know, but just to be sure: at() requires an index to exist, so it can only be used in places where this precondition is guaranteed. Other methods (like .value() and [] will return a default constructed value instead).
(Just for your information, I haven't seen any such cases in this commit nor have I been looking for it ;) )

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-kuhn are you sure? That's not the case for QgsFields, and QVector docs also doesn't state that (which QgsAttributes is derived from).

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 1969e09 Oct 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not for QgsFields (since that's our own implementation, but I think we may change that to better match the Qt implementation at any time)

QVector:

http://doc.qt.io/qt-4.8/qvector.html#at

i must be a valid index position in the vector (i.e., 0 <= i < size()).

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 1969e09 Oct 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... realized that [] has the same preconditions like at() as opposed to value()

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-kuhn yeah, so by my understanding [] should be harmlessly swappable to at() to force use of const.

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 1969e09 Oct 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like, yeah

Please sign in to comment.