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

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

Solution 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.

Solution 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());

Comments

[Dan]

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

[UndeadKernel]

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.

[José Tomás Tocino]

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