CODEWITHSUNDEEP
c++qtforeachqlist
Dan
Dan

Reputation: 5975

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

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

Upvotes: 43

Views: 58724

Answers (4)

BuvinJ
BuvinJ

Reputation: 11046

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

Upvotes: 0

Igor
Igor

Reputation: 27248

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

Upvotes: 56

alexisdm
alexisdm

Reputation: 29886

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

Upvotes: 15

Mat
Mat

Reputation: 206699

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.

Upvotes: 24

Related Questions