On Sat, 31 Jul 2004 18:58:25 +0100, Jonathan Ames <am**@reb.org> wrote:
Moving to C++ from Java, I'm still confused by some aspects of memory
cleanup operations.
For example, let's say I have a class MovingObject which maintains a
pointer to another class MovementAlgorithm.
MovingObject has a method
SetMovementAlgorithm(MovementAlgorithm* movementAlgorithm)
if the body of this method reads
{
this->movementAlgorithm = movementAlgorithm;
}
is this a memory leak every time a new MovementAlgorithm is set (other
than the first time)? Because the previous movement algorithm isn't
deleted?
There is no memory leak here because there is no memory allocated here,
it's the way that you *use* SetMovementAlgorithm that causes the memory
leak.
I think you are using it something like this
MovingObject x;
x.SetMovementAlgorithm(new MovementAlgorithm); // 1
....
x.SetMovementAlgorithm(new MovementAlgorithm); // 2
Here there is a memory leak, because the only pointer pointing to the
object allocated at step 1 has been overwritten at step 2, and so that
object can never be deleted.
But in this similar code there is no memory leak
MovingObject x;
MovementAlgorithm y1;
x.SetMovementAlgorithm(&y1);
....
MovementAlgorithm y2;
x.SetMovementAlgorithm(&y2);
What is the correct way to handle this problem?
It depends.
If I change the body of the method to read as follows :
if (this->movementAlgorithm != NULL) {
delete this->movementAlgorithm;
}
this->movementAlgorithm = movementAlgorithm;
I get program crash. All of the constructors for MovingObject
initialize the pointer variable either to NULL or through the new
operator. Can anyone explain this?
Not without seeing more code. I guess the most likely reason for the crash
is that you are deleteing the same memory twice because you have not
defined a suitable copy constructor or assignment operator for the class
MovingObject. When a MovingObject object is copied you are copying the
movementAlgorithm data member and hence end up deleting the same memory
twice. But that is only a guess.
The solution to all these problems is to use pointer in a less cavalier
fashion. I don't know exactly what you are tring to achieve but here are
some suggestions.
1) To you really need to use pointers at all? Could you not define
MovingObject like this
class MovingObject
{
...
MovementAlgorithm movementAlgorithm;
};
2) If you really do need pointers then read up on the 'rule of three'. as
soon as you include a pointer in a class you almost always need to define
proper copy constructors and assignment operators for that class. I
suspect that you are not doing that, although I don't know because you
didn't post the code.
3) As soon as you start allocating objects using new, then you must think
about whose responsiblity it is to free that pointer. In other words who
owns the pointer. The way you have described your MovingObject class so
far it sounds as if the MovementAlgorithm object is allocated outside the
MovingObject class but a MovingObject object takes ownership of the
pointer when SetMovementAlgorithm is called. Are you happy and clear about
that?
4) Learn about smart pointers, which take care of many of the problems in
correctly freeing allocated objects automatically. For instance have a
look at the shared_ptr class available from boost
http://www.boost.org/libs/smart_ptr/smart_ptr.htm. It could be exactly
what you need.
If all this doesn't help then post the definitions for Movingbject and
MovementAlgorithm and I'll take another look.
john
