84

I have this code:

int main()
{
    vector<int> res;
    res.push_back(1);
    vector<int>::iterator it = res.begin();
    for( ; it != res.end(); it++)
    {
        it = res.erase(it);
        //if(it == res.end())
        //  return 0;
    }
}

"A random access iterator pointing to the new location of the element that followed the last element erased by the function call, which is the vector end if the operation erased the last element in the sequence."

This code crashes, but if I use the if(it == res.end()) portion and then return, it works. How come? Does the for loop cache the res.end() so the not equal operator fails?

gsamaras
  • 71,951
  • 46
  • 188
  • 305
hidayat
  • 9,493
  • 13
  • 51
  • 66
  • Similar question: http://stackoverflow.com/questions/347441/erasing-elements-from-a-vector – Naveen Jan 10 '11 at 10:29
  • 7
    because this is just a simplification of the code, i am not trying to delete all the elements in the real code – hidayat Jan 10 '11 at 10:37

10 Answers10

167

res.erase(it) always returns the next valid iterator, if you erase the last element it will point to .end()

At the end of the loop ++it is always called, so you increment .end() which is not allowed.

Simply checking for .end() still leaves a bug though, as you always skip an element on every iteration (it gets 'incremented' by the return from .erase(), and then again by the loop)

You probably want something like:

 while (it != res.end()) {
        it = res.erase(it);    
 }

to erase each element

(for completeness: I assume this is a simplified example, if you simply want every element gone without having to perform an operation on it (e.g. delete) you should simply call res.clear())

When you only conditionally erase elements, you probably want something like

for ( ; it != res.end(); ) {
  if (condition) {
    it = res.erase(it);
  } else {
    ++it;
  }
}
Vikas Patidar
  • 42,865
  • 22
  • 93
  • 106
Pieter
  • 17,435
  • 8
  • 50
  • 89
  • ok so it first increment and after the incrementation it compares – hidayat Jan 10 '11 at 10:30
  • 1
    No, hidayat; your code is trying to delete all elements in the vector one by one. To do so, you should start at `res.begin()` and then never advance the iterator, but retrieve the iterator returned when erasing an element (the same goes for all STL containers). The increment itself is the part that's wrong. – Mephane Jan 10 '11 at 10:35
  • in the real code i am not trying to delete all the elements, but thanks, i understand what i did wrong now – hidayat Jan 10 '11 at 10:38
  • Hi , I am doing in the same way , but still I am getting "out_of_range" error. Can you please tell me why ? – DukeLover Feb 17 '16 at 07:28
  • @DukeLover you must do a `iterator++` while it's equal to `.end()` somewhere, without seeing any code that's all I can guess. If you can't figure it out, maybe pose a question? – Pieter Feb 17 '16 at 07:41
32
for( ; it != res.end();)
{
    it = res.erase(it);
}

or, more general:

for( ; it != res.end();)
{
    if (smth)
        it = res.erase(it);
    else
        ++it;
}
crazylammer
  • 1,152
  • 8
  • 7
3

Because the method erase in vector return the next iterator of the passed iterator.

I will give example of how to remove element in vector when iterating.

void test_del_vector(){
    std::vector<int> vecInt{0, 1, 2, 3, 4, 5};

    //method 1
    for(auto it = vecInt.begin();it != vecInt.end();){
        if(*it % 2){// remove all the odds
            it = vecInt.erase(it); // note it will = next(it) after erase
        } else{
            ++it;
        }
    }

    // output all the remaining elements
    for(auto const& it:vecInt)std::cout<<it;
    std::cout<<std::endl;

    // recreate vecInt, and use method 2
    vecInt = {0, 1, 2, 3, 4, 5};
    //method 2
    for(auto it=std::begin(vecInt);it!=std::end(vecInt);){
        if (*it % 2){
            it = vecInt.erase(it);
        }else{
            ++it;
        }
    }

    // output all the remaining elements
    for(auto const& it:vecInt)std::cout<<it;
    std::cout<<std::endl;

    // recreate vecInt, and use method 3
    vecInt = {0, 1, 2, 3, 4, 5};
    //method 3
    vecInt.erase(std::remove_if(vecInt.begin(), vecInt.end(),
                 [](const int a){return a % 2;}),
                 vecInt.end());

    // output all the remaining elements
    for(auto const& it:vecInt)std::cout<<it;
    std::cout<<std::endl;

}

output aw below:

024
024
024

A more generate method:

template<class Container, class F>
void erase_where(Container& c, F&& f)
{
    c.erase(std::remove_if(c.begin(), c.end(),std::forward<F>(f)),
            c.end());
}

void test_del_vector(){
    std::vector<int> vecInt{0, 1, 2, 3, 4, 5};
    //method 4
    auto is_odd = [](int x){return x % 2;};
    erase_where(vecInt, is_odd);

    // output all the remaining elements
    for(auto const& it:vecInt)std::cout<<it;
    std::cout<<std::endl;    
}
Jayhello
  • 5,931
  • 3
  • 49
  • 56
2

Something that you can do with modern C++ is using "std::remove_if" and lambda expression;

This code will remove "3" of the vector

vector<int> vec {1,2,3,4,5,6};

vec.erase(std::remove_if(begin(vec),end(vec),[](int elem){return (elem == 3);}), end(vec));
Niki Dimitrov
  • 86
  • 1
  • 5
1

The it++ instruction is done at the end of the block. So if your are erasing the last element, then you try to increment the iterator that is pointing to an empty collection.

Patrice Bernassola
  • 14,136
  • 6
  • 46
  • 59
0

The following also seems to work :

for (vector<int>::iterator it = res.begin(); it != res.end(); it++)
{
  res.erase(it--);
}

Not sure if there's any flaw in this ?

Skippy le Grand Gourou
  • 6,976
  • 4
  • 60
  • 76
  • While this code may answer the question, it is better to explain what it does and add some references to it. – Hamid Pourjam Dec 25 '15 at 13:03
  • 1
    I'm unsure about the code above. There are 3 main problems that I see. First, you don't sign back res.erase(it) back to `it` after removal. You must not have it++ inside for iterator statement while removing things thus you should have a conditional check to erase it. If the condition fails then you shall iterate to next (`it++`). Though I wonder why you have `it--`? Pardon me but why do you even decrement the iterator? Maybe I stumble, if that is the case I apologize. – knoxgon Sep 08 '17 at 00:51
  • @VG Thanks, I guess your comment addresses the question in the answer, therefore making it educational and maybe worth mentioning. I'm afraid I don't understand the logic of the `it--` either anymore, too much water has flowed under the bridge since then… – Skippy le Grand Gourou Sep 08 '17 at 10:01
  • @SkippyleGrandGourou Thanks for the reply, I haven't really found something that matches the decrement status above. Could that be the case to iterate back a step after removal? Maybe it is identical to `it = res.erase(it)`? Though I really doubt that. Hmmmm – knoxgon Sep 08 '17 at 10:18
  • @VG : According to Pieter's answer, "`res.erase(it)` always returns the next valid iterator". I guess `it--` and `it++` cancel, so to my understanding this piece of code keeps erasing the (new) first element. Perform `it--` doesn't seem a great idea, though, `it` now being the first element… – Skippy le Grand Gourou Sep 18 '17 at 12:02
  • @SkippyleGrandGourou Ah, I see, it makes perfect sense. Though `it--` doesn't seem safe at when `it` is the first element which would automatically be an invalid iterator. Thanks for clearing it out. – knoxgon Sep 18 '17 at 13:13
  • @VG Yes, that was the point of the end of my comment : `it--` is performed on the first element at *each* iteration… – Skippy le Grand Gourou Sep 18 '17 at 13:53
  • @SkippyleGrandGourou I was basically agree with you :) – knoxgon Sep 18 '17 at 15:29
  • doing `--` on the begin iterator causes undefined behaviour – M.M May 25 '20 at 07:09
  • 2
    This decrements the iterator after it is passed to the erase() but before erase() is executed. – Elias Apr 17 '22 at 18:17
  • @Elias: That's what is wanted... except for walking off the front of the container. – Ben Voigt Feb 06 '23 at 21:35
0

Do not erase and then increment the iterator. No need to increment, if your vector has an odd (or even, I don't know) number of elements you will miss the end of the vector.

Benoit
  • 76,634
  • 23
  • 210
  • 236
0

You increment it past the end of the (empty) container in the for loop's loop expression.

kbjorklu
  • 1,338
  • 8
  • 10
-1
if(allPlayers.empty() == false) {
    for(int i = allPlayers.size() - 1; i >= 0; i--)
    {
        if(allPlayers.at(i).getpMoney() <= 0) 
            allPlayers.erase(allPlayers.at(i));
    }
}

This works for me. And Don't need to think about indexes have already erased.

Dawoon Yi
  • 426
  • 3
  • 9
  • 1
    How can you say this works for you? You never tested that. This does not even compile. alllPlayers.at(i) does not return an iterator. But erase() expects an iterator. – Elmue Nov 14 '17 at 12:53
-1

As a modification to crazylammer's answer, I often use:

your_vector_type::iterator it;
for( it = res.start(); it != res.end();)
{
    your_vector_type::iterator curr = it++;
    if (something)
        res.erase(curr);
}

The advantage of this is that you don't have to worry about forgetting to increment your iterator, making it less bug prone when you have complex logic. Inside the loop, curr will never be equal to res.end(), and it will be at the next element regardless of if you erase it from your vector.

  • According to the spec (c++11), this is not cool. every iterator and reference after the point of erase is invalidated (23.3.6.5/3) Hence it (which you incremented before erase) is invalid after the erase. source: http://kera.name/articles/2011/06/iterator-invalidation-rules-c0x/ – Håkon Egset Harnes May 09 '18 at 14:37
  • Can you find a reference to the official spec that says that? I believe that website is inaccurate – Joseph Petroske May 10 '18 at 21:24
  • The latest openly available working draft for the standard I could find is for c++11, http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3242.pdf You can find the text at the point i referenced in my original comment. 23.3.6.5/3 "Effects: Invalidates iterators and references at or after the point of the erase" – Håkon Egset Harnes May 11 '18 at 13:47
  • @HåkonEgsetHarnes that is a pre-C++11 draft . See https://stackoverflow.com/questions/81656/where-do-i-find-the-current-c-or-c-standard-documents/83763#83763 – M.M May 25 '20 at 07:07
  • The code in this answer is wrong, erasing an item from a vector invalidates all iterators at the point of the erase and later (which includes `it`). https://en.cppreference.com/w/cpp/container/vector/erase – M.M May 25 '20 at 07:08