Properly destroying pointers in an std::map

Question

I have a map declared as

std::map<std::string, Texture*> textureMap;

which I use for pairing the path to a texture file to the actual texture so I can reference the texture by the path without loading the same texture a bunch of times for individual sprites. What I don't know how to do is properly destroy the textures in the destructor for the ResourceManager class (where the map is).

I thought about using a loop with an iterator like this:

ResourceManager::~ResourceManager()
{
    for(std::map<std::string, Texture*>::iterator itr = textureMap.begin(); itr != textureMap.end(); itr++)
    {
        delete (*itr);
    }
}

But that doesn't work, it says delete expected a pointer. It's pretty late so I'm probably just missing something obvious, but I wanted to get this working before bed. So am I close or am I totally in the wrong direction with this?

Answer 1

As far as your sample code goes, you need to do this inside the loop:

delete itr->second;

The map has two elements and you need to delete the second. In your case, itr->first is a std::string and itr->second is a Texture*.

If you need to delete a particular entry, you could do something like this:

std::map<std::string, Texture*>::iterator itr = textureMap.find("some/path.png");
if (itr != textureMap.end())
{
    // found it - delete it
    delete itr->second;
    textureMap.erase(itr);
}

You have to make sure that the entry exists in the map otherwise you may get an exception when trying to delete the texture pointer.

An alternative might be to use std::shared_ptr instead of a raw pointer, then you could use a simpler syntax for removing an item from the map and let the std::shared_ptr handle the deletion of the underlying object when appropriate. That way, you can use erase() with a key argument, like so:

// map using shared_ptr
std::map<std::string, std::shared_ptr<Texture>> textureMap;

// ... delete an entry ...
textureMap.erase("some/path.png");

That will do two things:

• Remove the entry from the map, if it exists
• If there are no other references to the Texture*, the object will be deleted

In order to use std::shared_ptr you'll either need a recent C++11 compiler, or Boost.

Answer 2

You're not using the right tool for the job.

Pointers should not "own" data.

Use boost::ptr_map<std::string, Texture> instead.

Answer 3

The answer didn't fully address the looping issue. At least Coverty (TM) doesn't allow erasing the iterator within the loop and still use it to continue looping. Anyway, after deleting the memory, calling clear() on the map should do the rest:

ResourceManager::~ResourceManager()
{
    for(std::map<std::string, Texture*>::iterator itr = textureMap.begin(); itr != textureMap.end(); itr++)
    {
        delete (itr->second);
    }
    textureMap.clear();
}