How to correctly initialize an object. [C++]

Question

I mentioned in one of my earlier questions that I'm reading book "C++ Coding Standards" By Herb Sutter and Andrei Alexandrescu. In one of the chapters they are saying something like this:

Always perform unmanaged resource acquisition, such as a new expression whose result is not immediately passed to a smart pointer constructor, in the constructor body and not in initializer lists.

Does that mean that I should use construction of this form (providing that data_3_ has to be initialized with new):

SomeClass(const T& value, const U& value2, const R& value3)
    : data_(value), data_2_(value2)
{
    data_3_ = new value3;
}

instead of:

SomeClass(const T& value, const U& value2, const R& value3)
    : data_(value), data_2_(value2), data_3_(new value3)
    // here data_3_ is initialized in ctor initialization list
    // as far as I understand that incorrect way according to authors
{
}  

Thanks in advance.

P.S. And if that's what they mean why are they using term unmanaged resource acquisition? I always thought that this resources are "manually managed"?

P.S 2. I'm sorry in advance if there are any formatting problems in this post - I have to admit - I absolutely detest the way of formatting on this forum.

Answer 1

The advice is necessary if the class contains two or more unmanaged resources. If allocation of one fails, then you will need to free all the previous allocated resources to avoid a leak. (EDIT: more generally, any exception thrown after allocating a resource has to be handled by deleting that resource). This can't be done if they are allocated in the initialiser list. For example:

SomeClass() : data1(new value1), data2(new value2) {}

will leak the value1 if new value2 throws. You will need to handle this, like so:

SomeClass() : data1(0), data2(0)
{
    data1 = new value1; // could be in the initialiser list if you want
    try
    {
        data2 = new value2;
    }
    catch (...)
    {
        delete data1;
        throw;
    }
}

Of course, all these shenanigans can be avoided by sensible use of smart pointers.

Answer 2

Initialization of manually managed resources may lead to resource leaks if the constructor throws an exception at any stage.

First, consider this code with automatically managed resources:

class Breakfast {
public:
    Breakfast()
        : spam(new Spam)
        , sausage(new Sausage)
        , eggs(new Eggs)
    {}

    ~Breakfast() {}
private:
    // Automatically managed resources.
    boost::shared_ptr<Spam> spam;
    boost::shared_ptr<Sausage> sausage;
    boost::shared_ptr<Eggs> eggs;
};

If "new Eggs" throws, ~Breakfast is not called, but all constructed members' destructors are called in reverse order, that is destructors of sausage and spam.

All resources are properly released, no problem here.

If you use raw pointers (manually managed):

class Breakfast {
public:
    Breakfast()
        : spam(new Spam)
        , sausage(new Sausage)
        , eggs(new Eggs)
    {}

    ~Breakfast() {
        delete eggs;
        delete sausage;
        delete spam;
    }
private:
    // Manually managed resources.
    Spam *spam;
    Sausage *sausage;
    Eggs *eggs;
};

If "new Eggs" throws, remember, ~Breakfast is not called, but rather the destructors of spam and sausage (which are nothing in this cause, because we have raw pointers as actual objects).

Therefore you have a leak.

The proper way of rewriting the code above is this:

class Breakfast {
public:
    Breakfast()
        : spam(NULL)
        , sausage(NULL)
        , eggs(NULL)
    {
        try {
            spam = new Spam;
            sausage = new Sausage;
            eggs = new Eggs;
        } catch (...) {
            Cleanup();
            throw;
        }
    }

    ~Breakfast() {
        Cleanup();
    }
private:
    void Cleanup() {
        // OK to delete NULL pointers.
        delete eggs;
        delete sausage;
        delete spam;
    }

    // Manually managed resources.
    Spam *spam;
    Sausage *sausage;
    Eggs *eggs;
};

Of course, you should instead prefer to wrap every unmanaged resource in a separate RAII class, so you can manage them automatically and group them together into other classes.