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

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 2

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 3

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);

  // ...