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

Question

Hi, 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

That's because a constructor may throw an exception.

In the situation you describe (ie not using a smart pointer), you have to free the resource in the destructor AND if the constructor throws an exceptions, with a try-catch block:

SomeClass(const T& value, const U& value2, const R& value3):data_(value),data_2_(value2) :
data_3_(NULL)
{
    try 
    {
        data_3_ = new value3;
    }
    catch(...)
    {
        delete data_3_;
        throw;
    }
}

.. Which you can't do if an exception is thrown in the initialisation list.

See this for further explanations.

Answer 2

It's a matter of exception safety.

If for some reason, the constructor fails and throws an exception, you can't clean up behind you.

Let's consider:

SomeClass() : data1(new T1()), data2(new T2()), data3(new T3()) {}

If T2's or T3s constructor throws, you definitely leak the memory corresponding to the initialization of data1. Also, you won't know which allocation raised the exception: was it new T2() or new T3()? and it's such a case you don't know whether it's safe to delete data2; as part of a constructor exception handler.

To write exception safe code, use smart pointers or use try/catch blocks in the body of the constructor.

SomeClass() : data1(new T1()), data2(new T2()), data3(new T3())
{
  data1 = new T1();
  try
  {
    data2 = new T2();
    try
    {
      data3 = new T3();
    }
    catch (std::exception&)
    {
      delete data2;
      throw;
    }
  }
  catch (std::exception&)
  {
    delete data1;
    throw;
  }
}

As you can see, using try/catch blocks is not that readable and likely error prone, compared to using member smart pointers.

Note: "C++ Coding Standards" Chapter 48 refers to "More Exceptional C++" Item 18 which itself refers to Stroustrup's "The Design And Evolution of C++3" Section 16.5 and Stroustrup's "The C++ Programming Language" Section 14.4.

EDIT: "More Exceptional C++ Item 18 has the same content as GotW #66: Constructor Failures. Refer to the web page in case you don't have the book.

Answer 3

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.

Answer 4

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 5

I'm posting this as an answer because it is too long to fit into a comment.

Consider:

A * a;
...
a = new A;

What happens if A's constructor throws?

• firstly, the A instance being constructed is never fully created
• that being the case, A's destructor will not be called - there is in fact no A object
• the memory allocated by new to build the A in is deallocated
• stack unwinding takes place, this means that the assignment to the pointer 'a' never takes place, it retains its original indeterminate value.

From this it should be obvious that there is nothing to call delete on, no allocated memory, no object of type A. The same logic holds if new throws, except the A constructor will never have been used in the first place.