Deleting a vector element in a loop, based upon a conditional statement

Question

In this code snippet Update() returns a boolean, if it returns false I would like to delete the element from the vector.

However, this produces a run-time error of debug assertion failed. The expression is "vector iterator not incrementable".

The code:

for(auto iter = someVector.begin(); iter != someVector.end(); ++iter){
    if(!iter->get()->Update()) iter = someVector.erase(iter);
}

I have tried subtracting from the iterator as follows too:

for(auto iter = particles.begin(); iter != particles.end(); ++iter){
    if(!iter->get()->Update()) iter = --(particles.erase(iter));
}

...but this results in "vector iterator not decrementable".

How can I make my code works as intended; so that the vector element is deleted when the Update() function returns false?

possible duplicate of [Remove elements of a vector inside the loop](http://stackoverflow.com/questions/8628951/remove-elements-of-a-vector-inside-the-loop) — Jerry Coffin, Aug 17 '13 at 19:12

score 5 · Accepted Answer · answered Aug 17 '13 at 19:08

5

Change the loop to this:

for(auto iter = someVector.begin(); iter != someVector.end();){
    if(!iter->get()->Update())
        iter = someVector.erase(iter);
    else
        ++it;
}

The reason for the assertion is that, after the call to erase, iter might be equal to end(). The iterator returned by erase is already "next", you're not supposed to increment it.

answered Aug 17 '13 at 19:08

jrok

54,456
9
109
141

this works perfectly, thank you! I didn't know you could omit the last part of the for loop either. – OMGtechy Aug 17 '13 at 19:22
No problem. Do read @Dietmar's answer, too! As for loop, you can omit all three statements: `for(;;) {}`. This is how some people like to write their infinite loops. – jrok Aug 17 '13 at 19:23
I feel educated! Thank you again. – OMGtechy Aug 17 '13 at 19:25

Dietmar Kühl · Answer 2 · 2013-08-17T19:28:24.710

1

I'd recommend not using erase() as above in the first place but rather use it something like this:

someVector.erase(std::remove_if(someVector.begin(), someVector.end(),
                                [](decltype(*someVector.begin()) element){
                                    return !element.get()->update();
                                },
                 someVector.end());

When just one element needs to be erased it does roughly the same as using the one iterator version of erase(). When multiple elements need to be erased, it does less copyies/moves. Note that I use a lambda function just because it is easier to express but the same can be done with a suitable function object if lambda functions are not available.

edited Aug 17 '13 at 19:28

answered Aug 17 '13 at 19:16

Dietmar Kühl

150,225
13
225
380

1

+1 I was writing this only :D, should be `return !element.get()->update()` – P0W Aug 17 '13 at 19:21
Thank you, it's always nice to know more than one way to solve a problem. However, in my case I will extremely rarely need to erase more than 1 element at a time. – OMGtechy Aug 17 '13 at 19:24
@JoshuaGerrard: Using the algorithm is no worse than erasing an element individually and works better in the rare case where there is more than 1 element. Thus, it is obviously the way to go! Not to mention that the algorithms could possibly wing a few extra performance improvements (e.g., suitable loop unrolling, due boundary checks just once in a debugging implementation, etc.): using `std::remove_if()` is a win! – Dietmar Kühl Aug 17 '13 at 19:31
Thanks again! I shall look into it more then, as the syntax looks rather confusing to me. If I can understand what is going on, I will use it. It's the [](decltype... that throws me. That said...google is my friend ;) – OMGtechy Aug 17 '13 at 19:34
@JoshuaGerrard: This is just a C++11 lambda function and a somewhat awkward use of `decltype()` to determine the argument type. You could also use a function object like `struct predicat { template bool operator()(T& o) { return !o.get()->Update(); } };` and then use `predicate()` instead. If the indirection via `get()` wouldn't be needed it would be easy to use `std::mem_fn()`. Using `std::bind()` to compose things would also work but would probably be more confusing... – Dietmar Kühl Aug 17 '13 at 19:38

Deleting a vector element in a loop, based upon a conditional statement

2 Answers2