Search code examples
c++pimpl-idiom

Best place to initialize default values in a pimpl class?


I make pretty extensive use of PImpl and something that I've found myself waffling on is where exactly to to initialize members of the Pimpl struct. Options are to create a constructor for the Private struct and initialize them there, or to initialize them in the main class's constructor.

myclass.hpp:

class MyClass {
public:
    MyClass();
    ~MyClass();
private:
    struct Private; unique_ptr<Private> p;
};

myclass.cpp:

#include "myclass.hpp"
#include <string>

struct MyClass::Private {
    int some_var;
    std::string a_string;

    // Option A
    Private() :
        some_var {42},
        a_string {"foo"}
    {}
};

MyClass::MyClass() : p(new MyClass::Private) {
    // Option B
    p->some_var = 42;
    p->a_string = "foo";
}

At present I don't really see a difference between the two other than if I were, for some reason, to want to create new Private objects or copy them around or something, then Option A might be preferable. It also is able to initialize the variables in an initialization list, for what that's worth. But, I find that Option B tends to be more readable and perhaps more maintainable as well. Is there something here I'm not seeing which might tilt the scales one way or the other?


Solution

  • By all means, follow the RAII approach and initialise members in your Private type. If you keep stuff local (and more importantly, at logical places), maintenance will thank you. More importantly, you will be able to have const members, if you use Option A.

    If you have to pass values from your MyClass ctor, then do create a proper constructor for Private:

    struct MyClass::Private {
        int const some_var; // const members work now
        std::string a_string;
    
        // Option C
        Private(int const some_var, std::string const& a_string) :
            some_var {some_var},
            a_string {a_string}
        {}
    };
    
    MyClass::MyClass() : p(new MyClass::Private(42,"foo")) {
    }
    

    Otherwise your Private members will be default constructed, only to be overwritten later (which is irrelevant for ints, but what about more complicated types?).