tags:

views:

380

answers:

5

I'm new to C++. I'd like to know how experienced coders do this.

what I have:

set<int> s;
s.insert(1);
s.insert(2);
s.insert(3);
s.insert(4);
s.insert(5);

for(set<int>::iterator itr = s.begin(); itr != s.end(); ++itr){
if (!(*itr % 2))
    s.erase(itr);
}

and of course, it doesn't work. because itr is incremented after it is erased. does it mean Itr has to point to the begin of the set everytime after i erase the element from the set?

+8  A: 

Erasing an element from std::set only invalidates iterators pointing to that element.

Get an iterator to the next element before erasing the target element.

Terry Mahaffey
+5  A: 

You don't need to go back to the start. set::erase only invalidates iterators that refer to the item being erased, so you just need to copy the iterator and increment before erasing:

for(set<int>::iterator itr = s.begin(); itr != s.end();)
{
    set<int>::iterator here = itr++;
    if (!(*here % 2))
        s.erase(here);
}
Mike Seymour
OK, I give up. What's the bug?
Mike Seymour
I was wrong, I thought you were skipping the first element. I take back my comment and downvote.
Terry Mahaffey
+6  A: 
 for(set<int>::iterator itr = s.begin(); itr != s.end(); ){
  if (!(*itr % 2))
      s.erase(itr++);

  else ++itr;
 }

effective STL by Scott Myers

pm100
You have extra bracket in code.
qba
why is itr++ allowed in the erase function, but not outside?
Quincy
`itr++` is allowed outside, but `++it` is in general preferable when the value is unused, for reasons way to tedious to go into every single time anyone does it ;-) In this case, it might be better to ignore the usual good practice and write `itr++`, just because the code reads slightly smoother if it's the same in both cases.
Steve Jessop
A: 

Why is there no std::set::erase_if?

FredOverflow
Probably belongs as a separate question. One often doesn't answer a question with a question.
GMan
You'd still have the invalid-iterator problem.
Mike DeSimone
@Mike: Fred probably means remove_if like std::list has, so you wouldn't need to code the loop yourself. Another side-question would be: why doesn't set's erase method behave like list's erase method by returning the iterator to next valid item?
UncleBens
Another question would be: why isn't this in `<algorithms>`?
Mike DeSimone
@Mike: `<algorithm>` by design only works with iterators without knowledge of the container class, hence it won't be able to call member functions like erase. Besides, removing is completely different with something node-based like set than with something array-like. One might write a couple of their own remove_if overloads for containers, using the remove-erase idiom for vector/deque, calling list.remove_if, or using this loop for set/map.
UncleBens
A: 

The best way is to use the combination of remove_if and erase

s.erase(remove_if(s.begin(), s.end(), evenOddFunctor), s.end())

This will be helpful http://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Erase-Remove

Also Refer to effective STL by scott meyers

Edit: Although my solution is wrong i am not deleting it. It might be a good learning for someone like me who does not about mutable/immutable iterators

Yogesh Arora
`remove_if` requires that `operator*` returns an non-const lvalue. std::set enforces that it's always ordered; returning a non-const lvalue from `std::set::operator*` would break that guarantee. Therefore `std::remove_if()` does not take `std::set::iterator`s
MSalters
Thanks i dint know that
Yogesh Arora
That was actually helpful. I've been stuck trying to do this with remove_if and this told me what the issue was. Thanks.
Ade Miller