How to properly iterate over container while modifying it?

Question

I want to modify my container while iterating over it. But the first version of the code didn't work properly. Can someone explain the reason?

// version 1
int main(){
    int a=1, b=2, c=5;
    std::set<int*> m = {&a, &b, &c};

    for(auto mi : m){
        std::cout << *mi << std::endl;
        m.erase(mi);
    }
}

Then I tried another version. It seems to be fine. Is there any potential trouble when I iterate like this?

// version 2
int main(){
    int a=1, b=2, c=5;
    std::set<int*> m = {&a, &b, &c};

    while (!m.empty()){
        std::cout << **m.begin() << std::endl;
        m.erase(m.begin());
    }

}

eerorika · Accepted Answer · 2020-01-09T15:36:10.980

Take a look at documentation of std::set::erase:

References and iterators to the erased elements are invalidated.

Which should be hardly surprising.

Now, think about what the range-based-for loop is shorthand for:

auto && range = range_expression ;
for (auto begin = begin_expr, end = end_expr; begin != end; ++begin) {
    range_declaration = *begin;
    loop_statement
}

Notice how the iterator to the current element is being used after the iteration, by the loop increment. The behaviour of incrementing an invalid iterator is undefined.

In the second loop, you always get a valid iterator by calling std::set::begin.

In this particular case, there is no point in modifying the container while iterating. Write one loop to print all elements, and then call clear to have the same result.

Usually there is a simpler way for specific cases, such as the one described above but in case there isn't, generally the correct way to modify a container while iterating is to not use a range-for but instead an explicit iterator loop and use the result of the operation (be it erase or insert) as the next iterator value instead of incrementing the now-invalid iterator.

score 1 · Answer 2 · answered Jan 09 '20 at 15:43

I don't see anything wrong with your second solution, you can use a set iterator though:

#include <iostream>
#include <set>

using namespace std;

int main(){
    int a=1, b=2, c=5;
    set<int*> m = {&a, &b, &c};
    set<int*>::iterator it = m.begin();

    while(it != m.end()){
        cout << **it << endl;
        it = m.erase(it);
    }
}

How to properly iterate over container while modifying it?

2 Answers2