C++: Erasing an iterator from a map and then incrementing to the next iterator

Question

This method causes an abort error: "map/set iterator not incrementable." Due to that after the if fails and a vaild iterator that should be erased is determined, (and is), continuing to the next iterator in the map via ++_iter fails because _iter is no longer a valid object/pointer.

What is the correct procedure for iterating through a map AND having the ability to remove individual items throughout?

typedef std::map<std::string, BITMAP*> MapStrBmp; typedef MapStrBmp::iterator MapStrBmpIter; \\... void BitmapCache::CleanCache() { //Clean the cache of any NULL bitmaps that were deleted by caller. for(MapStrBmpIter _iter = _cache.begin(); _iter != _cache.end(); ++_iter) { if(_iter->second != NULL) { if((_iter->second->w < 0 && _iter->second->h < 0) == false) continue; } _cache.erase(_iter); } }

@jrok: not applicable for std::map since its value_type is non copyable. — Yakov Galka
– Yakov Galka, Commented Jan 11, 2012 at 22:17
and more fundamentally because the elements of a map do not form a sequence. — Lara Bailey
– Lara Bailey, Commented Jan 11, 2012 at 22:20
See: stackoverflow.com/q/800955/237483, stackoverflow.com/q/180516/237483, stackoverflow.com/q/8234779/237483, stackoverflow.com/q/1038708/237483, — Christian Ammer
– Christian Ammer, Commented Jan 11, 2012 at 22:23

Yakov Galka · Accepted Answer · 2012-01-11 22:15:45Z

6

You just have to be a bit more careful:

void BitmapCache::CleanCache() { //Clean the cache of any NULL bitmaps that were deleted by caller. for(MapStrBmpIter _iter = _cache.begin(); _iter != _cache.end(); ) { if(_iter->second != NULL) { if((_iter->second->w < 0 && _iter->second->h < 0) == false) { ++_iter; continue; } } _cache.erase(_iter++); } }

answered Jan 11, 2012 at 22:15

Yakov Galka

72.8k16 gold badges148 silver badges226 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Rob Kennedy Over a year ago

Would you mind explaining with words the changes you made, please, so we don't have to play "spot the differences"?

v01pe Over a year ago

Why is incrementing the iterator inline in the erase command differnt to it be incremented on the next for loop? Just curious…

Mike Seymour Over a year ago

@v01pe: It's incremented before the call to erase invalidates it.

Yakov Galka Over a year ago

@v01pe: it's a postfix increment, so it increments the iterator to point to the next element, then passes a copy of the old iterator (that is to the item to be deleted) to _cache.erase. map::erase invalidates only iterators to elements that it actually deletes, so the current value of _iter remains valid and it points to the next element.

v01pe Over a year ago

@ybungalobill: I know that it's a postfix increment, but I didn't know, that a copy of the old value (iterator in that case) is evaluated. I thought the value it self is passed, and will be incremented at the end (not upfront) of the expression. Good to know!

Pablo · Accepted Answer · 2012-01-11 22:19:42Z

map::erase(iterator) gives you an iterator pointing to the next element in the map (if any) after erasing. Therefore, you can do:

for(MapStrBmpIter _iter = _cache.begin(); _iter != _cache.end(); ) { if(_iter->second != NULL) { if((_iter->second->w < 0 && _iter->second->h < 0) == false) { ++_iter; continue; } } _iter = _cache.erase(_iter); }

Unfortunately it's only since C++11. C++03 version returns void.
I think this is not even valid in C++11. The function prototype for erase in C++11 looks like this iterator erase( const_iterator pos ).

Kerrek SB · Accepted Answer · 2012-01-11 22:19:08Z

The standard erase loop for an associative container:

for (auto it = m.cbegin(); it != m.cend() /* not hoisted */; /* no increment */) { if (delete_condition) { m.erase(it++); } else { ++it; } }

MSN · Accepted Answer · 2012-01-11 22:19:34Z

The canonical way to safely erase iterators during an iteration is to use the result of container::erase:

void BitmapCache::CleanCache() { //Clean the cache of any NULL bitmaps that were deleted by caller. MapStrBmpIter _iter = _cache.begin(); while (_iter != _cache.end()) { bool erase_entry= true; if(_iter->second != NULL) { if((_iter->second->w < 0 && _iter->second->h < 0) == false) erase_entry= false; } if (erase_entry) _iter= _cache.erase(_iter); else ++_iter; } }

Collectives™ on Stack Overflow

C++: Erasing an iterator from a map and then incrementing to the next iterator

4 Answers 4

5 Comments

2 Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

5 Comments

2 Comments

Comments

Comments

Linked

Related