In 2018 with C++11 and higher, are helper init() functions considered bad form?

Question

Pre C++11 you had no non-static member initializion nor did you have construction delegation, so people often used private helper functions to help with initializtion to reduce code replication.

Is this good code in 2018?

class A  {
  int a1 = 0;
  double a2 = 0.0;
  string a3 = "";
  unique_ptr<DatabaseHandle> upDBHandle;

  void init(){
      upDBHandle = open_database(a1, a2, a3);
  }

public:
    A() { init(); }
    explicit A(int i):a1(i) {  init(); }
    explicit A(double d):a2(d) {  init(); }
    explicit A(std::string s):a3(std::move(s)) {  init(); } 
    A(int i, double d, std::string s) : a1(i), a2(d), a3(std::move(s)) { init(); }
};

How can this code be improved?

Answer 1

In my opinion, your code is fine. I try to avoid relying on subtle effects such as the member initialization order in constructor initialization lists. It violates DRY - you need to repeatedly use the same order: In the class body when declaring the members, and in the constructor initialization list. As soon as time goes by and the class becomes bigger, and you move the constructors into the .cpp file, things start getting more confusing. Therefore, I put things that require access to other members into init functions.

If the member is const, you can't do this. But then again, as the class author you can decide which member is const and which is not. Note that this is not to be confused with the anti-pattern of "construct, then init", because here the init happens within the constructor, and this is invisible to class users.

If you still mislike the use of init, I would advice against putting the call into the constructor initialization list. Perhaps for me an acceptable midway is to put it into the in-class initializer, and remove all calls from the constructors.

class A  {
  int a1 = 0;
  double a2 = 0.0;
  string a3 = "";
  unique_ptr<DatabaseHandle> upDBHandle = open_database(a1, a2, a3);

  // ...

Answer 2

C++ just isn't very good at dealing with multiple defaults. So doing this nicely is not going to be easy. There are different things you can do, but all of them have different trade-offs (e.g. scattering defaults around).

IMHO, the nicest solution that can be arrived at here, is one that isn't legal C++ (yet), but is a highly supported extension: designated initializers.

class A  {
  struct Params {
      int a1 = 0;
      double a2 = 0.0;
      string a3 = "";
  };
  Params p;
  unique_ptr<DatabaseHandle> upDBHandle;

public:
    explicit A(Params p_arg) 
      : p(std::move(p_arg))
      , upDBHandle(open_database(p.a1, p.a2, p.a3) { }
};

A a({});  // uses all defaults
A a2({.a2 = 0.5});  // specifies a2 but leaves a1 and a3 at default
A a3({.a1 = 2, .a2=3.5, .a3 = "hello"});  //  specify all

Answer 3

You could use the fact that, if a member is not initialized in a constructor's member initialization list, the default member initializer is executed. Moreover each member initialization is a full expression and the member initializations are always executed in the order of their declarations inside the class:

class A  {
  int a1 = 0;
  double a2 = 0.0;
  string a3 = "";
  unique_ptr<DatabaseHandle> upDBHandle = open_database(a1,a2,a3);
  //a1,a2 and a3 initializations are sequenced before
  //upDBHandle initialization.
public:
  //all these constructors will implicitly invoke upDBHandle's default initializer 
  //after a1,a2 and a3 inialization has completed.
  A() { }
  explicit A(int i):a1(i) {  }
  explicit A(double d):a2(d) {  }
  A(int i, double d, std::string s) : a1(i), a2(d), a3(std::move(s)) { }
};

Answer 4

Well, here's the thing, there is actually no need at all for init in your example. The language rules already dictate default member initializer go first, then what ever you do in the member initializer list. The members are initialized in declaration order. So you could really just define each c'tor as

A() upDBHandle(open_database(a1, a2, a3)) { }
explicit A(int i):a1(i), upDBHandle(open_database(a1, a2, a3)) {}
explicit A(double d):a2(d), upDBHandle(open_database(a1, a2, a3)) {}
explicit A(std::string s):a3(std::move(s)), upDBHandle(open_database(a1, a2, a3)) {} 
A(int i, double d, std::string s) : a1(i), a2(d), a3(std::move(s)), upDBHandle(open_database(a1, a2, a3)) {}

And that's it. Some may say it's not good, since changing the class declaration can make it break. But compilers are pretty good at diagnosing this. And I belong to the school of thought which says a programmer should know what they are doing, and not just build code by happy coincidence.

You can even do that pre-C++11. Sure, you don't have default member initializers, and would have to repeat the default values for each member (possibly name them somehow first, to avoid magic numbers), but that isn't related to the issue of initializing the member that depends on their initial value.

Advantages over init?

• Support for const members, however rare they may come.
• Support for objects with no default state. Those cannot be default initialized and wait until init is called.
• No need to add superfluous functions to your class, that are unrelated to its function.

Now, if the initialization code for the member is not trivial, you can still put it in a function. A free function, that is hopefully static to your classes' translation unit. Like so:

static std::unique_ptr<DatabaseHandle> init_handle(int a1, double a2, std::string const& a3) {
  // do other stuff that warrant a function block
  return open_database(a1, a2, a3);
}

A::A() upDBHandle(init_handle(a1, a2, a3)) { init(); }
A::A(int i):a1(i), upDBHandle(init_handle(a1, a2, a3)) {}
A::A(double d):a2(d), upDBHandle(init_handle(a1, a2, a3)) {}
A::A(std::string s):a3(std::move(s)), upDBHandle(init_handle(a1, a2, a3)) {} 
A::A(int i, double d, std::string s) : a1(i), a2(d), a3(std::move(s)), upDBHandle(init_handle(a1, a2, a3)) {}

That also means that you can have several of those for different members. So now the concern of initializing the members is more spread out.

Removing the need for the many c'tors can actually be done with something that Fred Larson suggested in his comment to your post.