C++: Should I initialize pointer members that are assigned to in the constructor body to NULL?

Question

Suppose I have:

// MyClass.h
class MyClass
{
  public:
    MyClass();

  private:
    Something *something_;
}

// MyClass.cpp
MyClass::MyClass()
{
  something_ = new Something();
}

Should I initialize something_ to NULL (or 0) in the constructor initialization list of the MyClass constructor? Or is that not necessary because I'm assigning to it in the body of the constructor? What is the recommended practice?

Emanuel

Why not just initialize the pointer in the first place? MyClass::MyClass() : something_(new Something()) – Ornithorhynchus

Oh, is that the best practice? – Emanuel

Always prefer initialization to assignment in the constructor body. And use a smart pointer like shared_ptr or scoped_ptr instead of a native pointer so that cleanup is automatic, even in the case of exceptions. – Ornithorhynchus

So if something_ is only used internally by MyClass, is it most appropriate to use shared_ptr or scoped_ptr? – Emanuel

Answer 1

Generally you only assign it once, in the initialization list or the body, unless the body initialization may or may not happen, or has prerequisite code:

MyClass::MyClass() 
{
   //this code must happen first
   // _now_ max is known
   something_ = new Something(max);
}

MyClass::MyClass() 
{
   if (max) 
       something_ = new Something(max);
   else
       something_ = NULL;
}

MyClass::MyClass() 
    : something_(new Something()) 
//as pointed out in the comments, use smart pointers
{
}

Curnin

Initializing a pointer data member with the result of a dynamic allocation in the initialization list is a bad idea. Too many things can go wrong. For example, if an exception is thrown during construction of some other data member, how do you know which data members were initialized so that you can destroy the dynamically allocated object? Even if you use a function-try block, you don't know where construction failed. (Of course, one should use a smart pointer, in which case dynamic allocation in the initialization list is okay, but beware when using ordinary pointers). – Overly

@James: Well, of course one should use a smart pointer instead of a native pointer. (Hah, you just edited your comment to say that.) – Ornithorhynchus

What if I'm creating to different objects and the first requires the second as an argument? Can you use an initializer in this case or is it the same as your max example? (e.g. : something_(new Something()), somethingElse_(new SomethingElse(something_))...) – Emanuel

@User: Initializers will run in the order the members are declared, NOT the order in which the initializers are declared. So if you have the members declared in the right order, you can have such dependencies. – Ornithorhynchus

To follow up on James's comment: I'd say that any class which stores the result of new in a raw pointer should do so at most once, and then it's OK to do it in the initializer list. Having more than one dynamic allocation doesn't scale in terms of exception correctness, and it's a sure-fire sign that you need to deploy a single-responsibility resource manager class. – Traveled

Answer 2

Generally speaking - no. But if somewhere in your constructor pointer is used before it has been initialized then you get undefined behaviour. However, the biggest problem that you have is this - if exception is thrown in constructor, its destructor is not called. So imagine you have two pointers to objects and allocation of the first object succeeds while the second allocation fails, in that case you end up with a resource leak. This can be solved by using "smart" pointers. But in that case they will be initialized in initialization list, and if you don't want to assign value to them twice then you better allocate memory in there rather than in constructor body.

Julietjulieta

I didn't think of the potential exception safety problem. – Curnin

Answer 3

Not necessary, especially if you immediately initialize in the constructor body, however if initialization is not so obvious then why not NULL it to avoid accidental access to uninitialized variable.

• minimal performance overhead
• saves so much debugging/troubleshooting time with crazy bug hunting

Trinomial

crazy bug hunting - definitely. I'm right now hunting for a resource leak (a Windows user object), and the most trouble make new calls in the initializer list: there is no before/after in the "atomic" world of initializer lists. – Adowa