Possible Logic Error: remove(vector.begin(),vector.end(),val)

Question

I am looking for a way to delete certain lines in a file. I read on Stack Overflow that the best way to do this is to write it into a vector, and then remove the element from the array, and then read the array back into the file. I was doing this, and was attempting to remove the elements of the array by using the remove method.

I found this method here:

Removing Method

But this didn't seem to work, and after compiling their code for myself, I found the theirs didn't work either.

Is there something simple I'm missing? Or is this incorrect. This is their code:

#include<bits/stdc++.h>
using namespace std;

int main(){
    vector<int> v;
    //Insert values 1 to 10
    v.push_back(20);
    v.push_back(10);
    v.push_back(30);
    v.push_back(20);
    v.push_back(40);
    v.push_back(20);
    v.push_back(10);

    vector<int>::iterator new_end;
    new_end = remove(v.begin(), v.end(), 20);

    for(int i=0;i<v.size(); i++){
        cout << v[i] << " ";
    }
    //Prints [10 30 40 10]
    return 0;
}

(For further clarification, 'didn't work' means: it printed 20 10 30 20 40 20 10)

Unrelated, but please take some time to read [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) and [Why is "using namespace std;" considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) — Some programmer dude, Feb 26 '23 at 19:15
As for your problem, please read about [the erase-remove idiom](https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom). — Some programmer dude, Feb 26 '23 at 19:16

score 3 · Accepted Answer · edited Feb 26 '23 at 19:23

Just change the for loop like

new_end = remove(v.begin(), v.end(), 20);

for ( auto first = v.begin(); first != new_end; first++){
    cout << *first << " ";
}

Or you could erase the removed elements like

v.erase( new_end, v.end() );
for(int i=0;i<v.size(); i++){
   cout << v[i] << " ";
}

or

v.erase( new_end, v.end() );
for ( const auto &item : v ){
   cout << item << " ";
}

Or you could combine the call of the standard algorithm std::remove with the member function erase like

v.erase( remove(v.begin(), v.end(), 20), v.end() );

Pay attention to that if your compiler supports C++ 20 then you can just write

std::erase( v, 20 );

score 2 · Answer 2 · answered Feb 26 '23 at 19:23

std::remove() doesn't actually remove matching elements from the specified range. It just moves them to the end of the range, and then returns an iterator to the first element that was "removed". You are ignoring that iterator.

To physically remove the matching elements from the std::vector, you need to call its erase() method afterwards, eg:

auto new_end = remove(v.begin(), v.end(), 20);
v.erase(new_end, v.end());

Otherwise, just use the iterator in your loop to stop iterating when you reach the "removed" elements, eg:

auto new_end = remove(v.begin(), v.end(), 20);
for(auto iter = v.begin(); iter != new_end; ++iter){
    cout << *iter << " ";
}

Possible Logic Error: remove(vector.begin(),vector.end(),val)

2 Answers2