I would like to erase all the items less than v in C++11 standard container set, here is my code:
void delete_less_than(set<int> & ss, int const v) { for (auto item: ss) { if (item < v) { ss.erase(ss.find(item)); } else break; } } Will the code work properly? I seems okay on my computer (g++ 4.7.3), but loops infinitely on some online judge where I submit my code.
That's not what the range-based loop is for. Don't use it; use a normal for loop instead. The range-based version is only if you want to do something with every element in the container, without mutating the container.
for (auto it = ss.begin(); it != ss.end(); ) { if (*it < v) { ss.erase(it++); } else { ++it; } } Even simpler:
ss.erase(ss.begin(), ss.lower_bound(v)); 
erase.
lower_bound solution is sexy!
ss.erase(it++); invalidates the it iterator. Is it okay, since it is incremented by the side-effect of it++ before ss.erase() is invoked, and hence the post-increment ++ is still operating on a valid iterator (and since iterators to non-erased elements are still valid after std::set::erase())? So, would swapping in two statements, ss.erase(it); ++it; then be using an invalid iterator? Could relying on a fine distinction like this be an issue for clarity or maintainability?
erase(it++) is fine; erase(it); ++it; is not fine, because erase invalidates the iterator.
ss.erase(it++); is clever but relying on pre-/post-increment semantics is setting up future maintainers for trouble. erase returns the next iterator, so the usual recommended solution is it = ss.erase(it);