Thread: STL map and dynamic memory

Personally I'd say use smart pointers (NOT std::auto_ptr, but boost::shared_ptr) instead of raw pointers inside the map and leave the memory handling to them.

Oh, I just realized that AH_Tze is right, there's a definite lack of encapsulation there. AnimFile should indeed delete its contained pointers by itself in the destructor.

boost smart pointers are good, but deleting memory is possible on std::vectors as well, and this is how I do it:

Code:

#include    <algorithm>    //Otherwise anything that uses these has to manually include this
template<class T>
struct VectorDeleter
{        
    void operator()(T * & obj)     //reference to a pointer, not a pointer to a reference 
    {            
        if(obj)        
        {            
            delete obj;                
            obj = NULL;    //I don't know if this actually does anything...hmm    
        }    
    }
};

//Deleter defined in GAMEEXPORT.H right now
#define    DELETE_MEMORY_AND_ERASE(a,t)    std::for_each(a.begin(),a.end(),VectorDeleter<t>());    a.clear();
#define    DELETE_INDEXED_MEMORY_AND_ERASE(vec,type,ptr)    std::for_each(ptr,ptr+1,VectorDeleter<type>());    vec.clear();

The algorithm header file is for std::for_each

the first macro deletes all of the dynamic memory on the vecotr, the second only deletes the memory at one index into the vector (so you can delete the i'th value in the array).

It's not immediately clear to understand, and I had some help generating this code so it's not like I 100% wrote this on my own.

You have macros and no exception safety. I prefer smart pointers...

Also, the if(obj) is useless.
And the second macro is hard to use. Given an index, I have to do:

Code:

DELETE_INDEXED_MEMORY_AND_ERASE(myvec, int*, myvec.begin()+index);

When I could do

Code:

delete myvec[index];
myvec[index] = 0;

which is shorter AND more readable.

>>yet visual studios library says yes.
Just a quick comment: I'm not sure what version of visual studio you have, but on mine it adds a disclaimer:
"Note that this return value is nonstandard behaviour."

How is the if(obj) useless? You may be right, but I dont' exactly understand why (maybe the syntax should be different, i.e the * and & should be switched, but I haven't had problems with it). Isn't that the equivalent of:

Code:

if(MyVec[0])
{
delete MyVec[0];
MyVec[0] = 0;
}

because

Code:

std::vector::operator[]

returns a reference, and doing

if(MyVec[0])

is checking whether the pointer is already nulled through a reference.

One other thing:
When I want to deallocate memory from a std::vector I typically also want to get rid of the raw pointer. So, doing what you do, you have to add a line of code something like:

Code:

std::vector<type>::iterator ptr = MyVec.begin();
delete MyVec[0]; //std::vector::operator [] returns a reference

MyVec[0] = 0;

MyVec.erase(ptr);

erase in itself doesn't delete memory, just raw pointers. So, the code I showed doesn't delete memory that's already nulled, and it also can get rid of pointers all in one step.

>>is checking whether the pointer is already nulled through a reference.
Indeed, and that's why it's bad The C++ language guarantees that deleting a NULL pointer will do nothing, so you're just wasting CPU cycles by adding an unnecessary branching statement.

>>When I want to deallocate memory from a std::vector
Just like to point out, the OP is dealing with a std::map..


Ted, have you updated your code (and are you still having the problem)? It would be helpful to see what you're working with at the moment

Well, you do have to delete the dynamic memory etc. as you would in a vector. On the other hand, as Codeplug mentioned you can't really erase elements in a std::map like you can in a std::vector.. so you'd probably just iterate through the map deleting all dynamic memory, and then when you're done you'd call clear() to remove all elements of the map. And that's where it's a little bit different Although, apparently, that isn't where the problem lies in Ted's code.