Feb 23

Consider the following class:

class Gadget
{
public:
    void MakeNewWidget()
    {
        delete widget_;
        widget_ = NULL;
        widget_ = new Widget;
    }

    const Widget& Get() { return *widget_; }

private:
    Widget* widget_;
};

In the MakeNewWidget() function the widget_ class member is first deleted and set to NULL before creating a new Widget object and assigning it to widget_. Plain and simple, and you see a lot of this kind of code around.

There’s a particular reasoning for this kind of code, especially in memory-contrived environments. The aim here is to prevent having a temporary Widget object in memory in addition to the widget_ member, thus saving on memory consumption.

But there’s something horribly wrong here. The “aim” of the code may be accurate, but it’s aiming straight at the coder’s foot. What would happen if the widget_ = new Widget; line threw an exception? The answer is that the Gadget object would be left in an inconsistent state, because the original widget_ member has already been deleted. If there’s a catch somewhere nearby down the call stack, the code there may well assume that the old state of the Gadget object is still valid; i.e. there’s been no change to its internals even though there was an exception.

We are able to remedy the situation by creating new Widget object into a temporary variable, deleting the original widget_ and assigning the temporary variable to widget_. Like follows:

void MakeNewWidget()
{
    Widget* temp = new Widget;
    delete widget_;
    widget = NULL;
}

Temporary variables such as these may not always be aesthetically pleasing, and we are using twice the amount of memory by briefly having two Widget objects present, but this code is significantly safer for the callers. No longer do the callers have to worry about the state of the Gadget object in case MakeNewWidget throws an exception, which is as it should be.

Leave a Reply

preload preload preload