Jump to content
  • Advertisement
Sign in to follow this  
sipickles

cleaning up a std::map and contents

This topic is 4816 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, I have a resource managing class which gets a bit messy during destruction. It has a std::map containing pairs of names and cTexture objects. Heres my destructor:
cResourceManager::~cResourceManager()
{
	std::map< cString, cTexture* >::iterator iter;
	for( iter = _textureList.begin(); iter != _textureList.end(); iter++ )
	{
		delete (*iter).second;
		_textureList[ (*iter).first ] = NULL;
	}
}

Am i going about this wrong? I get a debug assertion failure at this line: _ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)); Bit out my depth here, I admit! Any advice much appreciated, maps are a new area for me! Many Thanks Simon

Share this post


Link to post
Share on other sites
Advertisement
I would really advice you to at least take a look at smart pointers (boost).
http://www.boost.org/libs/smart_ptr/

And all of a sudden, all that damn memory management in C++ is almost gone.

Share this post


Link to post
Share on other sites
Manually setting the map's elements to NULL is a bad thing. Try this:

cResourceManager::~cResourceManager()
{
std::map< cString, cTexture* >::iterator iter;
for( iter = _textureList.begin(); iter != _textureList.end(); iter++ )
delete (*iter).second;
_textureList.clear();

}


Share this post


Link to post
Share on other sites
nothing wrong with setting the maps elements to null, though I would probably use (*iter).second = NULL rather than indexing the list again.

I do agree though, that unless you need to keep the entries in the map with null pointers to not bother and just clear the list, or use a boost::shared_ptr in the map and you can just call map.clear() and the memory will be cleaned up automatically.

Share this post


Link to post
Share on other sites
The clear is completely redundant since the maps destructor is about to be called. And the reason setting the pointers to NULL is potentially bad is because it can mask uninitialised variable errors elsewhere in your code. It is better to set them to some known and almost certainly bad value (i.e 0xBAADC0DE) to help identify uninitialised variable access later on, or not to touch them at all.

Enigma

Share this post


Link to post
Share on other sites

_textureList[ (*iter).first ] = NULL;


This seems to be grabbing the key, and then using it to index back into the map - what that gives you is, well, the value. So just (*iter).second = NULL would do the job; not that you need to, since the pointer itself will not persist anyway (it belongs to a node struct held internally by the std::map which will be cleaned up by the map's dtor, called implicitly).

Anyway, have you considered storing actual objects instead of pointers to the textures? The fact that you are willing to delete whatever's pointed at suggests that the map is supposed to "own" those textures, and there's no evidence of need for polymorphism here...

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!