12

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?

User
  • 62,498
  • 72
  • 186
  • 247
  • 8
    Why not just initialize the pointer in the first place? `MyClass::MyClass() : something_(new Something())` – Fred Larson Sep 27 '11 at 20:49
  • Oh, is that the best practice? – User Sep 27 '11 at 20:53
  • 3
    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. – Fred Larson Sep 27 '11 at 20:55
  • So if something_ is only used internally by MyClass, is it most appropriate to use shared_ptr or scoped_ptr? – User Sep 27 '11 at 21:07

3 Answers3

12

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
{
}
Mooing Duck
  • 64,318
  • 19
  • 100
  • 158
  • 5
    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). – James McNellis Sep 27 '11 at 20:56
  • 1
    @James: Well, of course one should use a smart pointer instead of a native pointer. (Hah, you just edited your comment to say that.) – Fred Larson Sep 27 '11 at 20:57
  • 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_))...`) – User Sep 27 '11 at 21:01
  • 2
    @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. – Fred Larson Sep 27 '11 at 21:03
  • 1
    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. – Kerrek SB Sep 27 '11 at 21:59
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.

1

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
Roman R.
  • 68,205
  • 6
  • 94
  • 158
  • *`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. – Wolf Oct 08 '15 at 09:40