2

Here's my code:

vector<int> v;
v.push_back(1);
v.push_back(2);
v.push_back(3);
v.push_back(4);
v.push_back(5);
v.push_back(6);
v.push_back(7);

for (vector<int>::iterator it = v.begin(); it != v.end(); ++it)
{
    if (*it == 7)
        v.erase(it);
    cout << *it << endl;
}

The issue is, the for loop does not stop running and prints garbage values. Can you give some insight on the issue...like maybe the last element stores information about size or the address of the end()?

Using the erase() method works fine for other locations in the vector, except the last element.

The console log shows garbage values like this:

34603778
35652354
50397954
34603592
34603536
34603536
34603536
34603536
34603794
36700688
34603536
34603536
34865684
51511824
34603536
34865680
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Huzaifa Rashid
  • 23
  • 3
  • 1
    You need an `else` there... if you've just erased at `(it)`, the behaviour of `*it` is undefined. Also, you've invalidated the iterator after modifying the container `(v)`, and subsequent use of `(it)` in the loop condition is also undefined behaviour. – Brett Hale Dec 15 '22 at 04:27
  • Does this answer your question? [Vector.erase(Iterator) causes bad memory access](https://stackoverflow.com/questions/2943912/vector-eraseiterator-causes-bad-memory-access) – Raymond Chen Dec 15 '22 at 04:32
  • @BrettHale I modified the code to use the else statement so that the invalidated "it" is not used at any other place in the for loop. The error is still the same. I think if we are removing the last element, it reduces the size of the vector. The current position becomes the end(). When the for loop iterates to the next position, it is not the end(), so the terminating condition of the for loop is not satisfied. Please do lemme know your further input on the problem – Huzaifa Rashid Dec 15 '22 at 04:43
  • 1
    @HuzaifaRashid - Paddy's answer below gives more details than my comment. It might also interest you to know that as of C++11 you can use initializer lists: `vector v {1,2,3,4,5,6,7};` or [`std::iota`](https://en.cppreference.com/w/cpp/algorithm/iota) after resizing the vector. – Brett Hale Dec 15 '22 at 04:51

1 Answers1

7

Your program has undefined behavior no matter what position you erase from in your vector. As the documentation says, the function:

Invalidates iterators and references at or after the point of the erase, including the end() iterator.

So, your iterator is dead the moment you erase. This is why the function returns a new iterator (to the item that is now in the position you just erased from). You must update your loop's iterator to use that.

The idiomatic way to erase from a standard container in a loop is to step the iterator in the loop body itself:

for (vector<int>::iterator it = v.begin(); it != v.end(); )
{
    if (*it == 7) {
        it = v.erase(it);
    } else {
        cout << *it << endl;
        ++it;
    }
}

However, this is not a great way in general to remove items from a vector. What if the vector contains lots of values to remove? Your operation becomes very inefficient, because each erase must shuffle the remaining items in the vector.

So the better approach is the erase-remove idiom:

v.erase(std::remove(v.begin(), v.end(), 7), v.end());

Since C++20, this is simplified further:

std::erase(v, 7);
paddy
  • 60,864
  • 6
  • 61
  • 103