Skip to content

Delete some items that were intended to be deleted. Avoid trying to delete items that don't exist

Toni Asensi Esteve requested to merge work/delete_items into master

Dear Dolphin developers, I was comparing the Krusader and Dolphin source code for undoing the closing of tabs when, in the «Clear all actions except the "Empty Recently Closed Tabs" action and the separator» part, I saw those differences:

        const int count = actions.size();
        for (int i = 2; i < count; ++i) {
            removeAction(actions.at(i));
        }
        const int quantActions = menu()->actions().size();
        for (int x = quantActions - 1; x >= 2; x--) {
            removeAction(menu()->actions().at(x));
        }

the key difference is in the for line. In the first case: there will be problems when the program tries to delete an item of the list that does no longer exist; also, some items won't be deleted from the list. It can be seen in that simple example:

    #include <QCoreApplication>
    #include <QDebug>

    int main(int argc, char *argv[])
    {
        QCoreApplication a(argc, argv);

        QList<int> list;
        for (int x = 0; x < 10; ++x) {
            list.append(x);
        }

        const int count = list.size();
        for (int i = 2; i < count; ++i) {
            qDebug() << "I'm going to delete the item number" << i << "when there are" << list.count();
            list.removeAt(i);
        }

        qDebug() << "There are" << list.count() << "items left. The expected quantity was: 2";
    }

The execution of that example shows:

    I'm going to delete the item number 2 when there are 10
    I'm going to delete the item number 3 when there are 9
    I'm going to delete the item number 4 when there are 8
    I'm going to delete the item number 5 when there are 7
    I'm going to delete the item number 6 when there are 6
    I'm going to delete the item number 7 when there are 6
    I'm going to delete the item number 8 when there are 6
    I'm going to delete the item number 9 when there are 6
    There are 6 items left. The expected quantity was: 2

However, using a for that is similar to the one used in Krusader, the item that is going to be deleted exists, and so problems aren't seen:

    #include <QCoreApplication>
    #include <QDebug>

    int main(int argc, char *argv[])
    {
        QCoreApplication a(argc, argv);

        QList<int> list;
        for (int x = 0; x < 10; ++x) {
            list.append(x);
        }

        const int count = list.size();
        for (int i = count - 1; i >= 2; i--) {
            qDebug() << "I'm going to delete the item number" << i << "when there are" << list.count();
            list.removeAt(i);
        }

        qDebug() << "There are" << list.count() << "items left. The expected quantity was: 2";
    }

Its execution shows:

    I'm going to delete the item number 9 when there are 10
    I'm going to delete the item number 8 when there are 9
    I'm going to delete the item number 7 when there are 8
    I'm going to delete the item number 6 when there are 7
    I'm going to delete the item number 5 when there are 6
    I'm going to delete the item number 4 when there are 5
    I'm going to delete the item number 3 when there are 4
    I'm going to delete the item number 2 when there are 3
    There are 2 items left. The expected quantity was: 2                    

That is the reason of this merge request :-). Thanks!

Edited by Toni Asensi Esteve

Merge request reports