13

I simply wanna erase the specified element in the range-based loop:

vector<int> vec = { 3, 4, 5, 6, 7, 8 };
for (auto & i:vec)
{
    if (i>5)
    vec.erase(&i);
}

what's wrong?

Mostafa Bahri
  • 2,303
  • 2
  • 19
  • 26

4 Answers4

27

You can't erase elements by value on a std::vector, and since range-based loop expose directly values your code doesn't make sense (vec.erase(&i)).

The main problem is that a std::vector invalidates its iterators when you erase an element.

So since the range-based loop is basically implemented as

auto begin = vec.begin();
auto end = vec.end()
for (auto it = begin; it != end; ++it) {
  ..
}

Then erasing a value would invalidate it and break the successive iterations.

If you really want to remove an element while iterating you must take care of updating the iterator correctly:

for (auto it = vec.begin(); it != vec.end(); /* NOTHING */)
{
  if ((*it) > 5)
    it = vec.erase(it);
  else
    ++it;
}  
Jack
  • 131,802
  • 30
  • 241
  • 343
17

Removing elements from a vector that you're iterating over is generally a bad idea. In your case you're most likely skipping the 7. A much better way would be using std::remove_if for it:

vec.erase(std::remove_if(vec.begin(), vec.end(),
                          [](const int& i){ return i > 5; }),
           vec.end());

std::remove shift the elements that should be removed to the end of the container and returns an iterator to the first of those elements. You only got to erase those elements up to the end then.

Alejandro
  • 3,040
  • 1
  • 21
  • 30
AliciaBytes
  • 7,300
  • 6
  • 36
  • 47
  • 5
    We now have [`std::experimental::erase_if`](http://en.cppreference.com/w/cpp/experimental/vector/erase_if). – T.C. Jul 10 '15 at 01:53
6

It's quite simple: don't use a range-based loop. These loops are intended as a concise form for sequentially iterating over all the values in a container. If you want something more complicated (such as erasing or generally access to iterators), do it the explicit way:

for (auto it = begin(vec); it != end(vec);) {
  if (*it > 5)
    it = vec.erase(it);
  else
    ++it;
}
Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
1

Actually it IS technically possible, and works in both MSVC & GCC. But it is inadvisable as it produces warning "C++20-style initializer statement in a range-based 'for' statement is nonstandard in this mode (3230)" This is probably because its ambigous whether there is an internal duplicate iterator generated by the ranged expression auto& i: ints, so which iterator is checked in the implied !=ints.end()?

#include <vector>
#include <iostream>
#include <algorithm>
using namespace std;

struct Int {
   Int(int i) : i(i) {} 
   ~Int(){ i = -i; }
   int i;
};

int  main() {
    vector<Int> ints;//{0,1,2,3,4};
    for (int i = 0; i<5; ++i)
      ints.emplace_back(Int(i));

    for (auto it = ints.begin();  auto& i: ints) { // requires C++20 ("ranged-based for with initializer")
      if (it->i == 3 || it->i == 2)
          ints.erase(it--); // Decrement after erasing a single element, and it preserves the iterator
       ++it;
    }
    for_each(
              ints.cbegin(),
              ints.cend(),
              [] (Int i) {cout << i.i << " ";} 
              );
   // outputs 0 1 4
}
Jonathan
  • 6,741
  • 7
  • 52
  • 69