C++ - Run a function before initializing a class member

Question

I have 2 resource managing classes DeviceContext and OpenGLContext both are members of class DisplayOpenGL. The resource lifetimes are tied to DisplayOpenGL. Initialization looks like this (pseudo code):

DeviceContext m_device = DeviceContext(hwnd);
m_device.SetPixelFormat();
OpenGLContext m_opengl = OpenGLContext(m_device);

The problem is the call to SetPixelFormat(), since I can't do that in the initializer list of the DisplayOpenGL c'tor:

class DisplayOpenGL {
public:
    DisplayOpenGL(HWND hwnd)
    : m_device(hwnd),
      // <- Must call m_device.SetPixelFormat here ->
      m_opengl(m_device) { };
private:
    DeviceContext m_device;
    OpenGLContext m_opengl;
};

Solutions that I can see:

• Inserting m_dummy(m_device.SetPixelFormat()) - Won't work as SetPixelFormat() has no retval. (should you do this if it had a retval?)
• Use unique_ptr<OpenGLContext> m_opengl; instead of OpenGLContext m_opengl;.
  Then initialize as m_opengl(), call SetPixelFormat() in the c'tor body and use m_opengl.reset(new OpenGLContext);
• Call SetPixelFormat() from DeviceContext c'tor

Which of these solutions is preferable and why? Anything I am missing?

I'm using Visual Studio 2010 Express on Windows, if it matters.

Edit: I'm mostly interested in the tradeoffs involved in deciding for one of these methods.

• m_dummy() doesn't work and seems inelegant even if it would
• unique_ptr<X> is interesting to me - when would I use it instead of a "normal" X m_x member? The two methods seem to be functionally more or less equivalent, except for the initialization issues.
• Calling SetPixelFormat() from DeviceContext c'tor certainly works but feels unclean to me. DeviceContext should manage the resource and enable its use, not impose some random pixel format policy on users.
• stijn's InitDev() looks like the cleanest solution.

Do I pretty much always want a smart pointer based solution in such cases anyway?

Answer 1

Comma operator to the rescue! An expression (a, b) will evaluate a first, then b.

class DisplayOpenGL {
public:
    DisplayOpenGL(HWND hwnd)
    : m_device(hwnd),
      m_opengl((m_device.SetPixelFormat(), m_device)) { };
private:
    DeviceContext m_device;
    OpenGLContext m_opengl;
};

Answer 2

Do I pretty much always want a smart pointer based solution in such cases anyway?

No. Avoid this unnecessary complication.

Two immediate approaches which have not been mentioned:

Approach A:

The clean way.

Create a little container object for m_device's storage which calls SetPixelFormat() in the constructor. Then replace DisplayOpenGL ::m_device with an instance of that type. Initialization order obtained, and the intent is quite clear. Illustration:

class DisplayOpenGL {
public:
    DisplayOpenGL(HWND hwnd)
        : m_device(hwnd),
            m_opengl(m_device) { }
private:
    class t_DeviceContext {
    public:
        t_DeviceContext(HWND hwnd) : m_device(hwnd) {
            this->m_device.SetPixelFormat();
        }
        // ...
    private:
        DeviceContext m_device;
    };
private:
    t_DeviceContext m_device;
    OpenGLContext m_opengl;
};

Approach B:

The quick & dirty way. You can use a static function in this case:

class DisplayOpenGL {
public:
    DisplayOpenGL(HWND hwnd)
    : m_device(hwnd),
      m_opengl(InitializeDevice(m_device)) { }
private:
    // document why it must happen this way here
    static DeviceContext& InitializeDevice(DeviceContext& pDevice) {
      pDevice.SetPixelFormat();
      return pDevice;
    }
private:
    DeviceContext m_device;
    OpenGLContext m_opengl;
};

Answer 3

First of all, you're doing it wrong. :-) It is very poor practice to do complex things in constructors. Ever. Make those operations functions on a helper object that must be passed into the constructor instead. Better is to construct your complex objects outside your class and pass them in fully created, that way if you need to pass them to other classes, you can do so into THEIR constructors at the same time as well. Plus that way you have a chance of detecting errors, adding sensible logging, etc.

class OpenGLInitialization
{
public:
    OpenGLInitialization(HWND hwnd)
        : mDevice(hwnd) {}
    void                 SetPixelFormat  (void)       { mDevice.SetPixelFormat(); }
    DeviceContext const &GetDeviceContext(void) const { return mDevice; }
private:
    DeviceContext mDevice;
};        

class DisplayOpenGL 
{
public:
    DisplayOpenGL(OpenGLInitialization const &ogli)
    : mOGLI(ogli),
      mOpenGL(ogli.GetDeviceContext())
      {}
private:
    OpenGLInitialization mOGLI;
    OpenGLContext mOpenGL;
};

Answer 4

Using uniqe_ptr for both seems appropriate here: you can forward declare DeviceContext and OpenGLContext, instead of including their headers, which is a good thing). Then this works:

class DisplayOpenGL
{
public:
  DisplayOpenGL( HWND h );
private:
  unique_ptr<DeviceContext> m_device;
  unique_ptr<OpenGLContext> m_opengl;
};

namespace
{
  DeviceContext* InitDev( HWND h )
  {
    DeviceContext* p = new DeviceContext( h );
    p->SetPixelFormat();
    return p;
  }
}

DisplayOpenGL::DisplayOpenGL( HWND h ):
  m_device( InitDev( h ) ),
  m_opengl( new OpenGLContext( *m_device ) )
{
}

If you can use c++11 you can replace InitDev() with a lambda.

Answer 5

If OpenGLContext has a 0 argument constructor and copy constructor you can change your constructor to

DisplayOpenGL(HWND hwnd)
: m_device(hwnd)
{
    m_device.SetPixelFormat();
    m_opengl = OpenGLContext(m_device);
};

unique_ptr is generally used when you want to make one of the members optional or "nullable," which you may or may not want to do here.

Answer 6

If it belongs to DeviceContext (and it seems so from your code), call it from DeviceContext c'tor.

Answer 7

Combine the Comma operator with a IIFE (Immediately-Invoked Function Expression), which lets you define variables and other complex stuff unavailable just with the comma operator:

struct DisplayOpenGL {
    DisplayOpenGL(HWND hwnd)
        : m_device(hwnd)
        , opengl(([&] {
            m_device.SetPixelFormat();
        }(), m_device))
    DeviceContext m_device;
    OpenGLContext m_opengl;
};

Answer 8

The comma operator would do pretty well in your case but I think this problem is a consequence of a bad planning of your classes. What I'd do is to let the constructors only initialize the state of the objects and not dependencies (such as OpenGL rendering context). I'm assuming that the OpenGLContext's constructor initialize the OpenGL Rendering Context and that's what I'd not do. Instead I'd create the method CreateRenderingContext for the OpenGLContext class to do the initialization and also to call the SetPixelFormat

class OpenGLContext {
public:
    OpenGLContext(DeviceContext* deviceContext) : m_device(deviceContext) {}
    void CreateRenderingContext() {
        m_device->SetPixelFormat();
        // Create the rendering context here ...
    }
private: 
    DeviceContext* m_device;
};

...

DisplayOpenGL(HWND hwnd) : m_device(hwnd), m_opengl(&m_device) {
    m_opengl.CreateRenderingContext();
}

Answer 9

Comma operator was the first thing I thought of. Constructor chaining also lets you declutter things a bit.

However I think I've come up with a way that's neater, makes your intentions clear, and creates less clutter around the real initialisation of your members - which is important when watching management of resources.

// Inherit from me privately.
struct ConstructorPreInitialisation{
    // Pass in an arbitrary lambda function here, it will simply be discarded
    // This remoes the need for a comma operator and importantly avoids cluttering your
    // real initialisation of member subobjects.
    inline ConstructorPreInitialisation( [[maybe_unused]] const auto λ ){ λ(); }
};
// WARN: This may increase the size of your class using it
// The compiler can probably elide this but from memory objects with zero length are not permitted
// Have not checked the fine details against the standard
// Therefore this should not be used if this is an unacceptable condition



// Usage
// Example class originally from: https://en.cppreference.com/w/cpp/language/constructor#Example

#include <fstream>
#include <string>
#include <mutex>
 
struct Base
{
    int n;
};   
 
struct Class : public Base, private ConstructorPreInitialisation
{
    unsigned char x;
    unsigned char y;
    std::mutex m;
    std::lock_guard<std::mutex> lg;
    std::fstream f;
    std::string s;
 
    Class(int x) 
      : Base{123}, // initialize base class
      
        ConstructorPreInitialisation([&](){
            // Call some global allocation here, for example.
        }),
        
        x(x),     // x (member) is initialized with x (parameter)
        y{0},     // y initialized to 0
        f{"test.cc", std::ios::app}, // this takes place after m and lg are initialized
        s(__func__), // __func__ is available because init-list is a part of constructor
        lg(m),    // lg uses m, which is already initialized
        m{}       // m is initialized before lg even though it appears last here
    {}            // empty compound statement
 
};
 

Available as gist here