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 — Jerry Coffin
– Jerry Coffin, Commented Aug 17, 2013 at 19:12

jrok · Accepted Answer · 2013-08-17 19:08:29Z

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, 2013 at 19:08

jrok

55.5k10 gold badges114 silver badges146 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

OMGtechy Over a year ago

this works perfectly, thank you! I didn't know you could omit the last part of the for loop either.

jrok Over a year ago

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.

OMGtechy Over a year ago

I feel educated! Thank you again.

Dietmar Kühl · Accepted Answer · 2013-08-17 19:28:24Z

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, 2013 at 19:28

answered Aug 17, 2013 at 19:16

Dietmar Kühl

155k18 gold badges238 silver badges395 bronze badges

5 Comments

P0W Over a year ago

+1 I was writing this only :D, should be return !element.get()->update()

OMGtechy Over a year ago

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.

Dietmar Kühl Over a year ago

@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!

OMGtechy Over a year ago

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

Dietmar Kühl Over a year ago

@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 <typename T> 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...

Collectives™ on Stack Overflow

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

2 Answers 2

3 Comments

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related