CODEWITHSUNDEEP
c++memorypointersnew-operator
Kaiser Wilhelm
Kaiser Wilhelm

Reputation: 618

c++ memory management

Is this a good way to delete a map of longs and objects made with new

// iterate over the map
for (std::map<unsigned long, Object*>::iterator it = objects.begin(), it_end = objects.end(); it != it_end; ++it)
{
    Object* temp = it->second;
    if(temp)
        delete temp;
}

// clear the map
objects.clear();

Upvotes: 1

Views: 284

Answers (6)

Mario The Spoon
Mario The Spoon

Reputation: 4859

Yes, although the better solution would be to use smart pointers instead of Object*, but that is a different subject.

You could shorten the content of the for to

{
   delete it->second;
}

delete null is defined and is a noop.

Upvotes: 4

Loki Astari
Loki Astari

Reputation: 264361

Yes. Use boost::ptr_map

boost::ptr_map<std::string, BigObject>   data;

data.insert("Plop", new BigObject);

When data goes out of scope it deletes all its value members.
Also for algorithms all members are returned as reference to the object (not pointer) thus it is much easier to use with the standard algorithms than a std::map<std::string, BigObject*> where you would need to de-reference the memebrs before use.

One would have to question why you have a map of pointer to int/long in the first place? Would it not be easier to just to store the value in the map?

Upvotes: 10

Sven
Sven

Reputation: 22673

I would recommend the use of a smart pointer, for example std::unique_ptr (this is a C++0x feature, not all compilers support it yet). For example:

std::map<unsigned long, std::unique_ptr<Object>> map;
// Do something with the map
map.clear(); // The objects are automatically deleted.

You can also use std::shared_ptr (or boost::shared_ptr if your compiler doesn't support C++0x smart pointers), which has the advantage that it will work if your map can contain the same pointer more than once, and your objects won't be destroyed if someone else still has a pointer to them.

boost::ptr_map is also an options, though I believe that, like the manual approach, will not work right if the map contains the same pointer more than once.

Upvotes: 1

Andy Finkenstadt
Andy Finkenstadt

Reputation: 3587

Yes. Regardless of what might be considered a design or architecture issue (the use of smarter pointers, for example), it is not unreasonable to use that method to clear the map.

One proviso: the objects map should not by changed by the destruction of the Object* pointers. If it is, it might be better to use something like this:

// iterate over the map
typedef std::map<unsigned long, Object*>::iterator map_iter;
for (map_iter it = objects.begin();
    it != objects.end();
    /* blank */ )
{
    iter_map to_delete = it;
    ++it;
    Object* temp = to_delete->second;
    if(temp)
        delete temp;
    objects.delete(to_delete);
}

Upvotes: 1

Mark Ransom
Mark Ransom

Reputation: 308111

It's a good start.

Once you've deleted the object, you should either remove the pointer from the map or set it to NULL to avoid dangling pointers. Edit: or of course you could just clear out the map when you're done, as your example shows.

When you store pointers in any of the standard containers, there's always a possibility that an exception or some code error will result in a memory leak. I'd suggest boost pointer containers as an alternative.

Upvotes: 2

MarkD
MarkD

Reputation: 4944

It is a perfectly fine way to delete.

Though you can make your life much easier by using a smart pointer.

Upvotes: 2

Related Questions