16

Can I remove elements from std::list, when I'm iterating on it? For example so:

std::list<int> lst;
//....
for (std::list<int> itr = lst.begin(); itr != lst.end(); itr++)
{
    if (*itr > 10)
         lst.remove(*itr);
}

? And why?

Mark Storer
  • 15,672
  • 3
  • 42
  • 80
Siarhei Fedartsou
  • 1,843
  • 6
  • 30
  • 44
  • 1
    possible duplicate of [Erasing items from an STL list](http://stackoverflow.com/questions/501962/erasing-items-from-an-stl-list) – sth Nov 23 '10 at 21:06
  • possible duplicate of [Can you remove elements from a std::list while iterating through it?](http://stackoverflow.com/questions/596162/can-you-remove-elements-from-a-stdlist-while-iterating-through-it) – AShelly May 03 '11 at 21:10

5 Answers5

36

The correct code is the following:

for (std::list<int>::iterator itr = lst.begin(); itr != lst.end(); /*nothing*/)
{
    if (*itr > 10)
        itr = lst.erase(itr);
    else
        ++itr;
}

When you delete an item from the list, you may invalidate the iterator (if it points to the item being deleted.) Therefore you need to delete using erase (which returns a valid iterator pointing to the next item).

Even better idea would be using std::remove_if:

bool greater_than_10(int x)
{
    return x > 10;
}

lst.remove_if(greater_than_10);

If your compiler supports lambdas, you can put it even shorter:

lst.remove_if([](int x){ return x > 10; });

(I didn't test this code, as my compiler is not so new; the lambda function is thankfully stolen from @John Dibling's answer.)


Actually, erasing from list invalidates only the iterators pointing to the item being deleted. Beware however that other STL containers don't have this property.


So, in short: generally speaking, you should not delete the items from the list while iterating through it, because the deletion may invalidate the iterator (and the program will possibly crash). If you are however completely sure that the items which you delete are not the values referenced by any of the iterators which you use at the moment of deletion, you may delete.

Beware that for the other STL containers (e.g. vectors) the constraint is even more strict: deleting from the container invalidates not only iterators pointing to the deleted item, but possibly other iterators, too! So deleting from that containers while iterating through them is even more problematic.

Vlad
  • 35,022
  • 6
  • 77
  • 199
9

No. The example code invalidates itr, causing undefined behavior. But this would work:

for (std::list<int>::iterator itr = lst.begin(); itr != lst.end(); )
{
    if (*itr > 10)
        itr = lst.erase(itr);
    else
        ++itr;
}
aschepler
  • 70,891
  • 9
  • 107
  • 161
  • Pre-incrementing the iterator is a good idea; I changed my code to match this. Now the example code is exactly the same :) – Vlad Nov 23 '10 at 21:22
3

No, you can't.

But you can (and should) use std::remove_if along with a functor that says "greater than 10`, like this:

#include <list>
#include <algorithm>


int main()
{
    std::list<int> lst;
    lst.push_back(1);
    lst.push_back(12);
    lst.push_back(1);
    //....
    lst.erase(std::remove_if(lst.begin(), lst.end(), std::bind2nd(std::greater<int>(), 10)), lst.end());
}

Another, more generic way to do this is to write your own custom functor. Here is a functor is_a_match that returns true if the value being checked is greater than 10. You can redefine operator() to return true to correspond to whatever it means in your case to "match":

#include <list>
#include <algorithm>
#include <functional>

struct is_a_match : public std::unary_function<int, bool>
{
    is_a_match(int val) : val_(val) {};
    bool operator()(int victim) const { return victim > val_; }
private:
    int val_;
};

int main()
{
    std::list<int> lst;
    lst.push_back(1);
    lst.push_back(12);
    lst.push_back(1);
    //....
    lst.erase(std::remove_if(lst.begin(), lst.end(), is_a_match(10) ));
}

If you have the benefit of a C++0x conformant compiler, you can also use lambdas, which makes it possible to get rid of the functor and write more expressive code in many cases

#include <list>
#include <algorithm>

int main()
{
    std::list<int> lst;
    lst.push_back(1);
    lst.push_back(12);
    lst.push_back(1);
    //....
    lst.erase(std::remove_if(lst.begin(), lst.end(), [](int v) {return v > 10;}));
}
John Dibling
  • 99,718
  • 31
  • 186
  • 324
  • Actually, `list` has it's own `remove_if` member function: `lst.remove_if(std::bind2nd(std::greater(), 10));` – Fred Larson Nov 23 '10 at 21:15
  • @Fred: True, but this is more generic and teaches how to do this for collections other than `list`. I also prefer non-member functions, as opposed to Scott Meyers and to an extend general thought. – John Dibling Nov 23 '10 at 21:17
  • @John: Agreed. I think that `list` provides some member functions like this (and `sort`, etc.) because they are more optimal for lists. It looks tidier than the erase/remove idiom too. But I appreciate your point. – Fred Larson Nov 23 '10 at 21:20
  • 1
    The erase-remove idiom is really a bad idea on lists. It slows removal of a single element down from O(1) to O(n). Don't do that. [Beware the illusion of container-independent code](http://ptgmedia.pearsoncmg.com/images/0201749629/items/item2-2.pdf). – fredoverflow Nov 23 '10 at 21:47
  • @Fred: meh. premature optimization. – John Dibling Nov 23 '10 at 22:01
  • @Fred: BTW I disagree with Meyers. He comes off as a professor with no real world experience. – John Dibling Nov 23 '10 at 22:03
  • 1
    @John: You really consider O(1) vs. O(n) *premature optimization*? You gotta be kidding me. The precise reason we use erase-remove on *vectors* is to get down from O(n^2) to O(n). Speedwise, choosing efficient algorithms is orders of magnitude more important than choosing C++ over "slower" languages. – fredoverflow Nov 23 '10 at 22:06
  • 1
    If you're creating a list of 10 integers once at program start-up, yeah, it doesnt matter. – John Dibling Nov 23 '10 at 22:43
0

I think you can, but you have to reassign the iterator after the removal of the element, which can be done with erasemethod instead that remove.

Otherwise it will be unsafe and shouldn't be done.

Jack
  • 131,802
  • 30
  • 241
  • 343
0

See http://www.cppreference.com/wiki/iterator/start for a description of iterators.

A couple of notes:

  • You should be using the pre-increment operator (++itr) instead of the post-increment operator (itr++)
  • Invalidation depends on the exact implementation of the iterator and its associated collection.
TreDubZedd
  • 2,571
  • 1
  • 15
  • 20