Hi. I am using an std::Vector and I have a question about memory allocation.

I have a vector of animation objects inside my graphic object. I add new animations externally via this method:

void Graphic::addAnimation( int* frames, AnimationStyle style, int frameCount, double updateInterval)
{
	Animation* a = new Animation(frames, style, frameCount, updateInterval);
	animations_.push_back(a);
}

Now, when I call the destructor should I do it this way...

Graphic::~Graphic()
{
	// Free all animations that have been allocated for this graphic.
	for ( unsigned int i = 0; i < animations_.size(); i++)
	{
		Animation* a = animations_[i];
		delete a;
	}
}

or this way...

Graphic::~Graphic()
{
	// Free all animations that have been allocated for this graphic.
	for ( unsigned int i = 0; i < animations_.size(); i++)
	{
		animations_.pop_back();
	}
}

All I need to do is call delete and give it the address of the object I am deleting right? I'm pretty sure with the first way I will end up with a vector full of dead pointers, but will the second way result in a memory leak or will the pop_back delete the memory for me?

pop_back() will call the destructor of whatever's contained in the vector. In this case, it calls the destructor of a pointer -- which does absolutely nothing! You need to explicitly destroy the objects pointed at by the elements of your vector, as you did in your first code sample. (However, a better way is to use some kind of 'smart' pointer type, to reduce the risk of programming errors that cause memory leaks.)

They are both slightly wrong.

You are correct that if you delete everything from the vector, you have a vector of dead pointers. Obviously, a simple animatins.clear() would solve that (after the loop).

The second form does not call delete on the pointer.

Why not this:

std::vector<Animation*>::iterator vecIter;
for(vecIter=animations_.begin();vecIter!=animations_.end();vecIter++)
   delete *vecIter;
animations_.clear();

Normally, a typedef is used for your storage class e.g. typedef std::vector<Animations*> animStore; and that way you can change your mind about using a list or a vector etc. [But it is useful to use erase instead of clear, since lists etc don't have clear]

You can get round the problem by using a reference counted pointer. [NOT auto_ptr: it will not work] But a suitable boost pointer (shared_ptr). Note that the boost shared pointers are now part of TR1.

I see, thanks for the tips! I notice people use lists a lot. It has always seemed beneficial to me to have random access to elements instead of using Iterators to find the element I want, but the amount of people that use linked lists makes me think they all know something I don't. Are they superior performance wise?

I see, thanks for the tips! I notice people use lists a lot. It has always seemed beneficial to me to have random access to elements instead of using Iterators to find the element I want, but the amount of people that use linked lists makes me think they all know something I don't. Are they superior performance wise?

The doubly linked list is one of the most useless data structures devised by mankind. Immutable, garbage-collected or reference-counted singly linked lists are very useful. Some of my opinion is based on years of not programming in C++, so maybe, in C++, doubly linked lists are more useful when dealing with memory management and avoiding unnecessary copying. There are some special purpose algorithms that can use doubly linked lists well.

This article has been dead for over six months. Start a new discussion instead.