Tell me how to correctly walk through the vector and remove some of its elements.
Ie, is it correct to perform such actions?

for (auto it = arr.begin(); it != arr.end(); it++) { if (check(*it) == false) arr.erase(it); } 

Or will there be a problem with the iterator after deleting the first element?


Read the comments, thank you all for your help.

I revised the code and realized that I did not quite complete the task:

 for (auto it = arr.begin(); it != arr.end(); it++) { if (check(*it) == false) { // сделать-что-то полезное с данными, которые потом будут удалены arr.erase(it); } } 

If you use the advice of @Harry, then the code can also be converted to this:

 arr.erase(remove_if(arr.begin(), arr.end(), [](auto x) { if (check(x)) { // сделать что-то полезное с данными из x return true; } return false; }), arr.end()); 

?

    4 answers 4

    Why not?

     arr.erase(remove_if(arr.begin(),arr.end(), [](auto x){ return !check(x); }), arr.end()); 

    ? As for me, it’s clearer and no problems with iterators ... It’s probably also faster than removing one element at a time ...

    If you rewrite check so that it works the other way around, then a lambda expression (or bind ) is not required.

    If we really need a loop, then use the fact that erase returns an iterator to the element following the deleted one:

     for(auto i = arr.begin(); i != arr.end(); i = (check(*i) == false) ? arr.erase(i) : i+1); 

    But remove_if that here the complexity is quadratic, unlike remove_if (thanks to @pavel).

    • the methods are really good, then I just have to make a double pass through the array, first to remove unwanted elements, and then to work with pleasing elements (in my implementation), but this is completely lethal, so I’m probably implementing it as you advise Zhihar
    • "If you rewrite check so that it works the other way around, then a lambda expression (or bind) is not required." - this is not really understood what is meant? - Zhihar
    • You check and delete with check(*it) == false . If you delete with newcheck(*it) == true , then the lambda-0 expression is not required, just pass the third parameter newcheck . - Harry
    • here is the loop in this form, I would not advise writing. Quadratic complexity out of the blue. - pavel
    • @pavel And for a vector in any form, if you remove one element at a time, it will be quadratic :) However, remove_if is also due to the movement of unreleased ones. - Harry

    erase will return the next iterator, the item will be deleted, but there will be a problem with the increment, so you need to add auxiliary code.

     for (auto it = arr.begin(); it != arr.end(); it++) { if (check(*it) == false){ it = arr.erase(it); --it; } } 

      After deleting, the iterator will become invalid and it ++ will not be possible to do it. In addition, at each iteration there will be a rebuilding of the vector, which is generally considered a heavy operation. You can transfer all the correct values ​​to the new vector:

       decltype(arr) new_vector; new_vector.reserve(arr.size()); for (auto i = 0; i < arr.size(); ++i) { new_vector.emplace_back(std::move(arrr[i])); } arr.swap(new_vector) 

        Alternative answer @Komodosh.

        In order not to decrement the iterator once again, you can do this:

         auto it = arr.begin(); while (it != arr.end()) { if (check(*it) == false) it = arr.erase(it); else it++; } 

        But it is better to listen to Harry and take erase-remove idiom.