Qt: is removing QList elements while iterating using foreach macro possible?

Question

I'm new to Qt and trying to learn the idioms.

The foreach documentation says:

Qt automatically takes a copy of the container when it enters a foreach loop. If you modify the container as you are iterating, that won't affect the loop.

But it doesn't say how to remove an element while iterating with foreach. My best guess is something like:

int idx = 0;
foreach (const Foo &foo, fooList) {
  if (bad(foo)) {
    fooList.removeAt(idx);
  }
  ++idx;
}

Seems ugly to have to scope idx outside the loop (and to have to maintain a separate loop counter at all).

Also, I know that foreach makes a copy of the QList, which is cheap, but what happens once I remove an element -- is that still cheap or is there an expensive copy-on-modify going on? Yes, deep copy happens.

EDIT : This doesn't seem like idiomatic Qt either.

for (int idx = 0; idx < fooList.size(); ) {
  const Foo &foo = fooList[idx];
  if (bad(foo)) {
    fooList.removeAt(idx);
  }
  else ++idx;
}

Neville

Why do you want to use foreach for this? – Psychoactive

@Mat, I don't have to use foreach, it just seems nice to use, and the docs seemed to suggest there was some way to do it. Qt seems so well thought out, I figured there'd be some obvious idiom for what I'm trying to do. – Neville

What about void QMutableListIterator::remove () ? developer.qt.nokia.com/doc/qt-4.8/… – Chery

Answer 1

You should better use iterators for that:

// Remove all odd numbers from a QList<int> 
QMutableListIterator<int> i(list);
while (i.hasNext()) {
    if (i.next() % 2 != 0)
        i.remove();
}

Octonary

Docs say "standard in Qt apps ... more convenient than STL ... slightly less efficient". OK, 2 out of 3 ain't bad. Thank you! – Neville

This answer correlates directly with Qt's description for using this class: doc.qt.io/qt-6/qmutablelistiterator.html#details – Disinherit

Answer 2

If you don't want a copy at all, use iterators. Something like:

QList<yourtype>::iterator it = fooList.begin();
while (it != fooList.end()) {
  if (bad(*it))
    it = fooList.erase(it);
  else
    ++it;
}

(And make sure you really want to use a QList instead of a QLinkedList.)

foreach is really nice when you want to traverse a collection for inspection, but as you have found, it's hard to reason about when you want to change the structure of the underlying collection (not the values stored in there). So I avoid it in that case, simply because I can't figure out if it is safe or how much copying overhead happens.

Psychoactive

You should not be afraid of using QList instead of QLinkedList in most situations. QList actually stores all its elements also as pointers. As such, appending, inserting or deleting elements is not as expensive as in a QVector. – Corpulent

Answer 3

If the test function is reentrant, you could also use QtConcurrent to remove the "bad" elements:

#include <QtCore/QtConcurrentFilter>
...
QtConcurrent::blockingFilter(fooList, bad);

Or the STL variant:

#include <algorithm>
...
fooList.erase(std::remove_if(fooList.begin(), fooList.end(), bad), 
              fooList.end());

Cripple

Works great with lambda functions, like blockingFilter(list, [](const Elem & e) { return e.shouldBeKept(); }); – Grieco

Answer 4

Here's an example spin off of the accepted answer, which I prefer purely for readabilty / style (I'm a fan of typedef's and for loops):

class MyClass 
{ 
... 
};
typedef QMutableListIterator<MyClass> MyClassIterator;

...

for( MyClassIterator it( myObjList ); it.hasNext(); )
{
    auto myObj( it.next() );
    if( isBad( myObj ) ) it.remove();
}

Disinherit