Should objects delete themselves in C++?

Generic Error picture Generic Error · Feb 7, 2009 · Viewed 20.9k times · Source

I've spent the last 4 years in C# so I'm interested in current best practices and common design patterns in C++. Consider the following partial example:

class World
{
public:
    void Add(Object *object);
    void Remove(Object *object);
    void Update();
}

class Fire : Object
{
public:
    virtual void Update()
    {
        if(age > burnTime)
        {
            world.Remove(this);
            delete this;
        }
    }
}

Here we have a world responsible for managing a set of objects and updating them regularly. Fire is an an object that might be added to the world under many different circumstances but typically by another object already in the world. Fire is the only object that knows when it has burned out so currently I have it deleting itself. The object that created the fire is likely no longer in existence or relevant.

Is this a sensible thing to do or is there a better design that would help clean up these objects?

Answer

abelenky picture abelenky · Feb 7, 2009

You have made Update a virtual function, suggesting that derived classes may override the implementation of Update. This introduces two big risks.

1.) An overridden implementation may remember to do a World.Remove, but may forget the delete this. The memory is leaked.

2.) The overridden implementation calls the base-class Update, which does a delete this, but then proceeds with more work, but with an invalid this-pointer.

Consider this example:

class WizardsFire: public Fire
{
public:
    virtual void Update()
    {
        Fire::Update();  // May delete the this-object!
        RegenerateMana(this.ManaCost); // this is now invaild!  GPF!
    }
}