C++ set container can't erase odd numbers

Question

This program asks the user for a number, then stores all numbers up to and including it into a set container. The problem is that I to erase specific numbers, but I can't because the program freezes whenever I try to erase an odd number (signified by NUMBER below). It works fine when I erase even numbers though. However, I noticed that if I change the initial value of y to an even number, I become unable to erase even numbers. Here I have it set to an odd number. What did I do wrong?

#include <iostream>
#include <set>
using namespace std;

int main()
{
set<int>s;
set<int>::iterator cnt;
int n,x,y=1;
cout<<"Number: ";
cin>>n;
for(x=0;x<n-1;x++)
{
    s.insert(y);
    y++;
}
for(cnt=s.begin();cnt!=s.end();cnt++)
{
    if(*cnt==NUMBER)
    s.erase(cnt);
}
for(cnt=s.begin();cnt!=s.end();cnt++)
cout<<*cnt<<"\n";
return 0;
}

Ugh - this is C++ not C, please put your variable declarations near their usage, or do this: `typedef set MySet; MySet s; /* ... */ for(MySet::iterator cnt = s.begin(); cnt != s.end(); ++cnt)` — kfsone, Sep 17 '13 at 04:40

David Schwartz · Answer 1 · 2013-09-17T04:46:51.317

The documentation for set::erase says "Iterators, pointers and references referring to elements removed by the function are invalidated. All other iterators, pointers and references keep their validity."

for(cnt=s.begin();cnt!=s.end();cnt++)
{
    if(*cnt==NUMBER)
    s.erase(cnt);
}

As soon as you do s.erase(cnt), cnt no longer refers to an element of the set. So when you do cnt++, you're trying to make it point to the "next element", but there is no next element.

One possible solution:

cnt = s.begin();
while (cnt != s.end())
{
    if (*cnt == NUMBER)
        s.erase(cnt++);
    else
        ++cnt;
}

score 1 · Answer 2 · answered Sep 17 '13 at 05:41

std::set has an erase that takes a key value instead of an iterator, so your entire removal process can be reduced to just: s.erase(NUMBER);.

You can also use std::copy to get the data from the set to output, so you end up with:

 s.erase(NUMBER);
 std::copy(s.begin(), s.end(),
           ostream_iterator<int>(cout, "\n"));

Alternatively, you could filter out the undesired value when you copy the output from the set to standard output, something like:

std::remove_copy(s.begin(), s.end(),  
                 ostream_iterator<int>(cout, "\n"), NUMBER);

C++ set container can't erase odd numbers

2 Answers2