Removing item from vector, while in C++11 range 'for' loop?

Question

I have a vector of IInventory*, and I am looping through the list using C++11 range for, to do stuff with each one.

After doing some stuff with one, I may want to remove it from the list and delete the object. I know I can call delete on the pointer any time to clean it up, but what is the proper way to remove it from the vector, while in the range for loop? And if I remove it from the list will my loop be invalidated?

std::vector<IInventory*> inv;
inv.push_back(new Foo());
inv.push_back(new Bar());

for (IInventory* index : inv)
{
    // Do some stuff
    // OK, I decided I need to remove this object from 'inv'...
}

Answer 1

No, you can't. Range-based for is for when you need to access each element of a container once.

You should use the normal for loop or one of its cousins if you need to modify the container as you go along, access an element more than once, or otherwise iterate in a non-linear fashion through the container.

For example:

auto i = std::begin(inv);

while (i != std::end(inv)) {
    // Do some stuff
    if (blah)
        i = inv.erase(i);
    else
        ++i;
}

Answer 2

Every time an element is removed from the vector, you must assume the iterators at or after the erased element are no longer valid, because each of the elements succeeding the erased element are moved.

A range-based for-loop is just syntactic sugar for "normal" loop using iterators, so the above applies.

That being said, you could simply:

inv.erase(
    std::remove_if(
        inv.begin(),
        inv.end(),
        [](IInventory* element) -> bool {
            // Do "some stuff", then return true if element should be removed.
            return true;
        }
    ),
    inv.end()
);

Answer 3

You ideally shouldn't modify the vector while iterating over it. Use the erase-remove idiom. If you do, you're likely to encounter a few issues. Since in a vector an erase invalidates all iterators beginning with the element being erased upto the end() you will need to make sure that your iterators remain valid by using:

for (MyVector::iterator b = v.begin(); b != v.end();) { 
    if (foo) {
       b = v.erase( b ); // reseat iterator to a valid value post-erase
    else {
       ++b;
    }
}

Note, that you need the b != v.end() test as-is. If you try to optimize it as follows:

for (MyVector::iterator b = v.begin(), e = v.end(); b != e;)

you will run into UB since your e is invalidated after the first erase call.