I have a QStandardItemModel with a single column (represents a list). Each item in the list has a unique integer ID stored as the QStandardItem's data (via QStandardItem::setData which I guess is into Qt::UserRole+1 by default).
Given one of these IDs, I'd like to find and remove the corresponding row from the model. Right now I'm doing this:
void NetworkManager::removeSessionFromModel (QStandardItemModel *model, int sessionId) {
foreach (const QStandardItem *item, model->findItems("*", Qt::MatchWildcard)) {
if (item->data() == sessionId) {
model->removeRow(item->index().row());
break;
}
}
}
It works fine, but every single line of that function makes me cringe. Is there a cleaner way to do any of this?
You're using findItems wrong, it can already return the item you want just by passing the value you're searching for. If you call it like you're doing right now you're looping through your items at least two times, since findItems must iterate through all the items to find those that match your pattern, in your case all items match, then you iterate the returned items again to find the sessionId.
void NetworkManager::removeSessionFromModel (QStandardItemModel *model, int sessionId) {
auto items = model->findItems(QString::number(sessionId));
if (!items.empty()) {
auto row = items.first()->index().row();
model->removeRow(row);
}
}
Alternatively you can use the match method since findItems uses that internally, so you avoid allocating the StandardItem just to get its index. Also match returns right after the number of items matching the pattern, in this case the value of sessionId, are found so it doesn't always iterate all the items; that's more efficient. Obviously if the value is not found after iterating all the items it returns an empty list.
auto start = model->index(0, 0);
auto indexes = model->match(start, Qt::UserRole + 1, QString::number(sessionId), 1, Qt::MatchExactly);
if (!indexes.empty()) {
auto row = indexes.first().row();
model->removeRow(row);
}