map question

Hello all,

I have the following code:

typedef map<string, CNodeData*, less<string> > NodeData;

NodeData _datalist;

say I populate _datalist, then how can I iterate through it and delete all the CNodeData items?? So I dont have memory leaks.

Thanks!

Code:

---

for(NodeData::iterator it = _datalist.begin(); it != _datalist.end(); ++it)
{
  delete it->second;
}

_datalist.clear();

---

Dont I need to set them to NULL also to avoid dangling pointers??

Thanks!

Cilu's code works good and no need to set NULL

Code:

---

delete 0;

---

is correct operation.

fyi ... using pointers as either the key or value in a map can
be tricky. Let's look at a simple example:

Code:

---

template <typename C>
void DeleteValueAndClear(C & c)
{
    typename C::iterator it;
    for (it=c.begin(); it!=c.end(); ++it)
    {
        delete it->second;
    }

    c.clear();
}

//
std::map<int,float*> m;
m.insert( std::make_pair(5,new float) );
m.insert( std::make_pair(1,new float) );
m.insert( std::make_pair(3,new float) );
m.insert( std::make_pair(2,new float) );

DeleteValueAndClear(m);

---

No memory leak in the above code.

But change the insert code as follows :

Code:

---

std::map<int,float*> m;
m.insert( std::make_pair(5,new float) );
m.insert( std::make_pair(1,new float) );
m.insert( std::make_pair(1,new float) );
m.insert( std::make_pair(2,new float) );

---

The second insert with a key==1 will fail (key already exists in the map).
So there will only be 3 elements in the map. So only 3 deletes will be
done.

One possibility is to add everything thru a function (not tested). If the
insert fails, you would delete the pointer then.

Code:

---

template<typename C, typename Key , typename Value>
bool InsertPointerValue(C & c , const Key & key , Value * value)
{
    if (!c.insert(std::make_pair(key,value)).second)
    {
        delete value;
        return false;
    }
    return true;
}

//

std::map<int,float*> m;

InsertPointerValue(m,5,new float);
InsertPointerValue(m,1,new float);
InsertPointerValue(m,1,new float);
InsertPointerValue(m,2,new float);

DeleteValueAndClear(m);

---

Well, it looks like cilu assumed your intentions were to clear the map<> of all elements. But after re-reading your post, I guess that's not what you're asking. So yes, feel free to set 'it->second' to NULL to avoid dangling pointers.

You may need to ask yourself the question: Should the map<> container be responsible for allocating and freeing CNodeData objects?

If not, you might want to consider using CNodeData & references instead of pointers. This will change your code design so that the map<> is not responsible for memory management, but merely contains a reference. You will need to find some other suitable location to allocate and later free up the pointers. This will avoid the possibility of NULL pointers and hopefully dangling pointers (references?).

Quote:

---

Originally Posted by lab1

Dont I need to set them to NULL also to avoid dangling pointers??

Thanks!

---

Well, it depends how you use them. If you copied that pointers in some variables, than you will have problems yes. If you use them solely from _datalist, no, since as you can see, I added a call to clear() at the end, which removes all elements from the map.

And you really should consider the use of smart pointers to take a lot
of the worry out of memory leaks.


boost has a shared pointer. Josuttis has a smart pointer in his book
on the C++ Standard Library.