CODEWITHSUNDEEP
c++constructorinitializationsmart-pointersinitialization-list
Christian Aichinger
Christian Aichinger

Reputation: 7227

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

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:

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.

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

Upvotes: 34

Views: 15036

Answers (9)

stellarpower
stellarpower

Reputation: 367

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

Upvotes: -1

Mateus Sarmento
Mateus Sarmento

Reputation: 191

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

Upvotes: -1

Bruno Martinez
Bruno Martinez

Reputation: 2956

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

Upvotes: 1

Jason H
Jason H

Reputation: 17

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

Upvotes: 0

justin
justin

Reputation: 104698

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

Upvotes: 7

MSN
MSN

Reputation: 54604

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

Upvotes: 43

stijn
stijn

Reputation: 35901

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.

Upvotes: 0

Dan
Dan

Reputation: 13160

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.

Upvotes: 0

SomeWittyUsername
SomeWittyUsername

Reputation: 18358

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

Upvotes: -1

Related Questions