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, Jan 11 '12 at 22:17
and more fundamentally because the elements of a map do not form a sequence. — CB Bailey, Jan 11 '12 at 22:20
See: http://stackoverflow.com/q/800955/237483, http://stackoverflow.com/q/180516/237483, http://stackoverflow.com/q/8234779/237483, http://stackoverflow.com/q/1038708/237483, — Christian Ammer, Jan 11 '12 at 22:23

score 6 · Accepted Answer · answered Jan 11 '12 at 22:15

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 '12 at 22:15

Yakov Galka

70,775
16
139
220

Would you mind explaining with words the changes you made, please, so we don't have to play "spot the differences"? – Rob Kennedy Jan 11 '12 at 22:29
Why is incrementing the iterator inline in the erase command differnt to it be incremented on the next for loop? Just curious… – v01pe Jan 11 '12 at 23:14
@v01pe: It's incremented *before* the call to `erase` invalidates it. – Mike Seymour Jan 12 '12 at 00:58
1

@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. – Yakov Galka Jan 12 '12 at 10:16
@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! – v01pe Jan 12 '12 at 10:30

score 2 · Answer 2 · answered Jan 11 '12 at 22:19

2

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);
}

answered Jan 11 '12 at 22:19

Pablo

8,644
2
39
29

1

Unfortunately it's only since C++11. C++03 version returns `void`. – Yakov Galka Jan 11 '12 at 22:24
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 )`. – gibraltar Jul 02 '15 at 16:15

score 1 · Answer 3 · answered Jan 11 '12 at 22:19

1

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;
    }
}

answered Jan 11 '12 at 22:19

Kerrek SB

464,522
92
875
1,084

score 0 · Answer 4 · answered Jan 11 '12 at 22:19

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;
    }
}

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

4 Answers4