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?

Stratus

This seems like the kind of case where a static factory function may be more useful than a constructor. – Reforest

seems to me like your third solution would work. Is there some reason you have chosen not to? Also, why not use a library like GLFW to load your openGL context? – Trivia

I was not aware of GLFW when I started. Will have a look, thanks. – Stratus

Another solution is to use the comma operator: https://mcmap.net/q/437402/-c-run-a-function-before-initializing-a-class-member – Enenstein

@Reforest What would a factory function provide here? Wouldn’t it still need to call a constructor itself? – Polyphonic

@Kröw Sure, but the constructor could be private. The static factory function's body would look pretty much exactly like OP's first code block, and then pass off m_device and m_opengl to the private constructor. – Reforest

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

Enenstein

What if OpenGLContext constructor takes more than 1 argument? – Kauppi

I would imagine you would do m_opengl((m_device.SetPixelFormat(), m_device), arg2) – Cleisthenes

Is there a syntax where the comma operator can still be used when the member variable's constructor doesn't accept any parameter? In the above example something like m_opengl((m_device.SetPixelFormat(), ) doesn't work due to "error: expected primary-expression before ')' token". I end up using a helper variable calling an ad-hoc lambda relying on the initialization order. It works, but it doesn't feel clean (see example code godbolt.org/z/Mxcns5hsG). – Lardaceous

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

Mister

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

Untread

In general, I would say that what should go to a constructor isn't about complexity, but instead about invariants. Do you have a reference for your claim? I do, for mine: isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-ctor – Shirlyshiroma

Requiring more for the caller to do, for no reason other than “it was bad practice that the API handle it in the constructor” is terrible advice. – Polyphonic

Answer 4

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

Rebutter

Works well, although it looks like the lambda isn’t needed here, since the comma operator would allow the SetPixelFormat call anyway. – Polyphonic

@Kröw You are right. The lambda can declare variables, though. – Rebutter

Answer 5

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.

Yoshi

Answer 6

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.

Holliehollifield

Answer 7

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

Rootlet

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

Wallacewallach

Wouldn’t this mean the OP’s code will now have to call CreateRenderingContext instead of SetPixelFormat? – Polyphonic

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

Regent