Monday, June 24, 2013

std::map insert, stuff I should have already known

Spot the error in the code:

typedef std::map<std::string, CacheEntry*> CacheMap;
typedef CacheMap::iterator CacheIterator;

void doSomething(CacheMap& cacheMap, std::string const& id)
{
    CacheIterator iter = cacheMap.find(id);
    if (iter != cacheMap.end())
    {
        CacheEntry* originalEntry = iter->second;

        CacheEntry* switchedCacheEntry = convertEntry(iter->second);

        delete originalEntry;

        cacheMap.insert(std::make_pair(id, switchedCacheEntry));

    }
}

There are actually a few errors in this code snippet if you are taking into account exception safety. The first error regarding exception safety is performing a delete before keeping the state of the map intact. In C++, destructors can never throw exceptions; therefore deleting the originalEntry is not a big deal. BUT what if the delete succeeds and then the cacheMap.insert throws? The cacheMap is left in an errant state because the entry in the map points at a deleted object. That's not good. The remedy for that error is simple, rearrange the order of the insertion and deletion. There is still another exception safety problem with this in that the switchedCacheEntry object will be dangling but that could be fixed by other means.

The big error that I uncovered when writing code like this was the use of std::map::insert. To me insert means that the element being inserted overwrites the contents in the map; but this is NOT THE CASE [1]. 


"Because element keys in a map are unique, the insertion operation checks whether each inserted element has a key equivalent to the one of an element already in the container, and if so, the element is not inserted, returning an iterator to this existing element (if the function returns a value)."


So there it is. In this case, it will be more efficient to just overwrite the entry in the map rather than attempt an insert. Using an insert in this case will cause us to keep the original entry object in the map instead of our switchedCacheEntry. 


REFERENCES


No comments:

Post a Comment