Finding / removing a row from a QStandardItemModel by item data

Question

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?

Answer 1

How about traversing the QStandardItemModel directly? Something like this:

void NetworkManager::removeSessionFromModel (QStandardItemModel *model, int sessionId) 
{
    for (int i = 0; i < model->rowCount(); ++i)
    {
        if (model->item(i)->data() == sessionId)
        {
            model->removeRow(i);
            break;
        }
    } 
}

Not sure how QStandardItemModel behaves with random access, maybe your method is more efficient.

Edit:

Actually, there is a function to do what you want: QAbstractItemModel::match

It returns a QModelIndexList with all entries that have matching data in a given role.

void NetworkManager::removeSessionFromModel (QStandardItemModel *model, int sessionId)
{
    QModelIndexList list = model->match(model->index(0, 0), Qt::UserRole + 1, sessionId);

    if (!list.empty())
        model->removeRow(list .first().row());
}

Setting data to a specific role can be done as follows:

model->setData(model->index(row, col), QVariant(13), Qt::UserRole + 1);

Answer 2

You need to get the row index from your item id.

A more effective way could be to use a QMap with the row index as value and the item id as a key.

In this case, you also need to maintain the map values every time you add/remove a row.

If you don't have 3 millions items in your list, just keep it simple and use your code. By optimize this code, you probably also add complexity and reduce maintainability, and you get is 0,05 ms instead of 0,06 ms.

In GUI code, I often have code like this : it's simple, everyone get it immediatly and it does the job. It' also fast enough.

Answer 3

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);
}