Fun with STL std::map Iterators and erase()
Today I got a somewhat sketchy bug report that, if it was true, would possibly have pointed to a section in my code where I was deleting things from an STL std::map using the erase() method whose argument was an iterator on the std::map. The original code looked something like this:
InstrumentAliasMap::iterator ia = mInstruments.begin(); while (ia != mInstruments.end()) { if (ia->first == anInstrument) { // found a match - erase it mInstruments.erase(ia); } else { // no match, check the next one ++ia; } }
where my thinking was that the iterator would be updated to the next, valid element after the erase. I think that was a major boo-boo. In all honesty, I didn't really know, but it seemed to test OK, and so it went. The problem is that after the call to erase() the iterator, ia, is invalid and can't be moved to the next valid element in the iteration.
I did a little searching on this as I was sure there was something simple about this. What I read was that the right way to do this was to take advantage of the properties of the post-increment operator which will increment the value (move it to the next element) but return the original value. So the code becomes:
InstrumentAliasMap::iterator ia = mInstruments.begin(); while (ia != mInstruments.end()) { if (ia->first == anInstrument) { // found a match - erase it mInstruments.erase(ia++); } else { // no match, check the next one ++ia; } }
In this code, the iterator is moved to the next valid element and yet the original iterator value is returned to the argument of erase(). This then removes the element from the map and we're sitting on the next, good one.
I like this a lot more, and it's clear what I should have done all along. I won't forget this guy.