4
\$\begingroup\$

This Question now has a follow-up here

I don't think that there is any new technique in my code but in my opinion it's ok all in all (with a bit of magic). Maybe you can find something to improve or provide some hints on what's good and what's bad about this wrapper class.

XoursWindow.h

#ifndef __XOURSWINDOW_H
#define __XOURSWINDOW_H

#include <Windows.h>

class XoursWindow {

public:
    XoursWindow ();
    XoursWindow (XoursWindow&);
    XoursWindow& operator=(XoursWindow);

    ~XoursWindow ();

    ATOM registerWindowClass (HINSTANCE);           // registers standard class for XOURSWINDOW
    ATOM registerWindowClass (WNDCLASSEX*);         // registers custom class with WNDCLASSEX
    ATOM registerWindowClass (wchar_t*, HINSTANCE); // registers class with custom class name

    void setWindowClassAtom (ATOM);
    ATOM getWindowClassAtom ();

    void createWindow (DWORD, wchar_t*, DWORD, int, int, int, int, HWND, HMENU, HINSTANCE, int);
    void createWindow (wchar_t*, int, int, int, int, HINSTANCE, int);
    void createWindow (wchar_t*, HINSTANCE, int);

    int destroyWindow ();

    void setAdditionalData (void*);     // setter for additional window data
    void* getAdditionalData ();         // getter for additional window data

    WNDPROC onCreate;
    WNDPROC onPaint, onEraseBkgnd;

    WNDPROC onSize, onMove;
    WNDPROC onGetMinMaxInfo;

    WNDPROC onCommand, onNotify;
    WNDPROC onTimer;

    WNDPROC onClose, onDestroy, onQuit;

    WNDPROC eventHandler;

    HWND getWindowHandle ();
    int getLastError ();

private:
    struct OpaquePtr;
    OpaquePtr *optr;
};

#endif // __XOURSWINDOW_H

XoursWindow.cpp:

#include "header/XoursWindow.h"
#include <CommCtrl.h>
#include <stdio.h>

enum XWINDOW_ERROR {
    XWINDOW_SUCCESS = 1,
    XWINDOW_FAILURE,

    /*  registerWindowClass
    */  //  ERRORS
    XWINDOW_WNDCLASS_ATOM_ALREADY_SET,

    /*  setWindowClassAtom
    */  //  ERRORS
    XWINDOW_SET_WNDCLASSATOM_FAILED,

    /*  createWindow
    */  //  WARNINGS
    XWINDOW_WNDCLASS_ALREADY_EXISTS,

    //  ERRORS
    XWINDOW_WINDOW_ALREADY_CREATED,
    XWINDOW_CREATING_WINDOW_FAILED
};

struct XoursWindow::OpaquePtr {
    wchar_t className[128];
    ATOM classAtom;

    static unsigned long objectCounter;
    DWORD objectId = 0;
    void* windowData = NULL;

    HWND window = NULL;
    XWINDOW_ERROR xwnd_err = XWINDOW_SUCCESS;

    static bool deepcopy_object (XoursWindow*, XoursWindow&);
    void unset (XoursWindow*);

    static LRESULT CALLBACK initialProc (HWND, UINT, WPARAM, LPARAM);                                   // callback function for window initializing
    static LRESULT CALLBACK messageForwardingProc (HWND, UINT, WPARAM, LPARAM, UINT_PTR, DWORD_PTR);    // subclassing routine for message forwarding to eventProc

    LRESULT CALLBACK eventProc (XoursWindow*, UINT, WPARAM, LPARAM);                                    // intern event handler

    static LRESULT CALLBACK eventHandling (HWND, UINT, WPARAM, LPARAM);                                 // standard event handler, per default: eventHandler = eventHandling

    /* this callback method will be deleted in release version
    let's call it an undocumented possibility to have ultimate freedom */
    static LRESULT CALLBACK preCreationEventHandling (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
        return DefWindowProc (hwnd, msg, wParam, lParam);
    };
};

unsigned long XoursWindow::OpaquePtr::objectCounter = 0;

XoursWindow::XoursWindow () {
    this->optr = new OpaquePtr;
    this->optr->unset (this);
}

XoursWindow::XoursWindow (XoursWindow& origin) {
    if (XoursWindow::OpaquePtr::deepcopy_object (this, origin))
        origin.optr->unset (&origin);       // unset origin to avoid unqualified use
    else {
        this->optr->unset (this);           // when deep copy failed unset this to avoid unqualified use
        this->optr->xwnd_err = XWINDOW_FAILURE;
    }

}

XoursWindow& XoursWindow::operator=(XoursWindow origin) {       // !!! IMPROVE! (exception handling, etc.)
    if (XoursWindow::OpaquePtr::deepcopy_object (this, origin))
        origin.optr->unset (&origin);       // unset origin to avoid unqualified use 
    else {
        this->optr->unset (this);           // when deep copy failed unset this to avoid unqualified use
        this->optr->xwnd_err = XWINDOW_FAILURE;
    }

    return *this;
}

XoursWindow::~XoursWindow () {
    delete this->optr;
}

bool XoursWindow::OpaquePtr::deepcopy_object (XoursWindow *dest, XoursWindow& src) {
    dest->onCreate = src.onCreate;
    dest->onPaint = src.onPaint;
    dest->onEraseBkgnd = src.onEraseBkgnd;
    dest->onSize = src.onSize;
    dest->onMove = src.onMove;
    dest->onGetMinMaxInfo = src.onGetMinMaxInfo;
    dest->onCommand = src.onCommand;
    dest->onNotify = src.onNotify;
    dest->onTimer = src.onTimer;
    dest->onClose = src.onClose;
    dest->onDestroy = src.onDestroy;
    dest->onQuit = src.onQuit;
    dest->eventHandler = src.eventHandler;

    dest->optr = new OpaquePtr (*(src.optr));
    if (src.optr->window == dest->getWindowHandle () && src.optr->window != NULL)
        SetWindowLongPtr (dest->getWindowHandle (), GWLP_USERDATA, (LONG_PTR) dest);
    else
        return false;

    return true;
}

void XoursWindow::OpaquePtr::unset (XoursWindow *ownerInstancePtr) {        /* unset object data (window/window class will not be destroyed!) */
    ownerInstancePtr->onCreate = NULL;
    ownerInstancePtr->onPaint = ownerInstancePtr->onEraseBkgnd = NULL;

    ownerInstancePtr->onSize = ownerInstancePtr->onMove = NULL;
    ownerInstancePtr->onGetMinMaxInfo = NULL;

    ownerInstancePtr->onCommand = ownerInstancePtr->onNotify = NULL;
    ownerInstancePtr->onTimer = NULL;

    ownerInstancePtr->onClose = ownerInstancePtr->onDestroy = ownerInstancePtr->onQuit = NULL;

    ownerInstancePtr->eventHandler = XoursWindow::OpaquePtr::eventHandling;

    XoursWindow::OpaquePtr::objectCounter++;  // won't be decremented (currently)
    ownerInstancePtr->optr->objectId = 1000 + XoursWindow::OpaquePtr::objectCounter;
    ownerInstancePtr->optr->windowData = NULL;

    ownerInstancePtr->optr->classAtom = 0;
    memset (ownerInstancePtr->optr->className, 0, sizeof (wchar_t) * 128);
    lstrcpy (ownerInstancePtr->optr->className, L"XoursWindow Standard Window Class\0");

    ownerInstancePtr->optr->window = NULL;
    ownerInstancePtr->optr->xwnd_err = XWINDOW_SUCCESS;
}

ATOM XoursWindow::registerWindowClass (HINSTANCE hInst) {       /* standard window class registration */
    this->optr->xwnd_err = XWINDOW_SUCCESS;
    if (this->optr->classAtom != 0){
        this->optr->xwnd_err = XWINDOW_WNDCLASS_ATOM_ALREADY_SET;
        return 0;
    }

    WNDCLASSEX wincl;
    wincl.hInstance = hInst;
    wincl.lpszClassName = this->optr->className;
    wincl.lpfnWndProc = XoursWindow::OpaquePtr::initialProc;
    wincl.style = CS_DBLCLKS;
    wincl.cbSize = sizeof (WNDCLASSEX);
    wincl.hIcon = LoadIcon (NULL, IDI_APPLICATION);
    wincl.hIconSm = LoadIcon (NULL, IDI_APPLICATION);
    wincl.hCursor = LoadCursor (NULL, IDC_ARROW);
    wincl.lpszMenuName = NULL;
    wincl.cbClsExtra = 0;
    wincl.cbWndExtra = sizeof (XoursWindow*);
    wincl.hbrBackground = (HBRUSH) COLOR_BACKGROUND + 1;

    return (this->optr->classAtom = RegisterClassEx (&wincl));
}

ATOM XoursWindow::registerWindowClass (WNDCLASSEX* wincl) {     /* register user defined window class (with limits) */
    this->optr->xwnd_err = XWINDOW_SUCCESS;
    if (this->optr->classAtom != 0) {
        this->optr->xwnd_err = XWINDOW_WNDCLASS_ATOM_ALREADY_SET;
        return 0;
    }

    /* force the use of our window callback procedure to ensure magic */
    wincl->lpfnWndProc = XoursWindow::OpaquePtr::initialProc;
    wincl->cbWndExtra = sizeof (XoursWindow*);
    return (this->optr->classAtom = RegisterClassEx (wincl));
}

ATOM XoursWindow::registerWindowClass (wchar_t* className, HINSTANCE hInst) {       /* standard window class registration with user defined class name */
    this->optr->xwnd_err = XWINDOW_SUCCESS;
    if (this->optr->classAtom != 0){
        this->optr->xwnd_err = XWINDOW_WNDCLASS_ATOM_ALREADY_SET;
        return 0;
    }

    memset (this->optr->className, 0, sizeof (wchar_t) * 128);
    lstrcpy (this->optr->className, className);

    return registerWindowClass (hInst);
}

void XoursWindow::setWindowClassAtom (ATOM wndClass) {          /* alternate way for setting a window class */
    this->optr->xwnd_err = XWINDOW_SET_WNDCLASSATOM_FAILED; /* pre-set error to save a few code lines */

    /* find window handle for retrieving class name */
    HWND tmpWnd = FindWindow (MAKEINTATOM (wndClass), NULL);
    if (tmpWnd == NULL)
        return;

    /* prepare buffer and get class name */
    wchar_t *buffer = new wchar_t[128];
    if (GetClassName (tmpWnd, buffer, 128) != 0) {
        this->optr->xwnd_err = XWINDOW_SUCCESS;

        this->optr->classAtom = wndClass;
        memset (this->optr->className, 0, sizeof (wchar_t) * 128);
        lstrcpy (this->optr->className, buffer);
    }

    delete[] buffer;
}

ATOM XoursWindow::getWindowClassAtom () {
    return this->optr->classAtom;
}

void XoursWindow::createWindow (DWORD dwStyleEx, wchar_t* strTitle, DWORD dwStyle, int posX, int posY, int width, int height, HWND hWndParent, HMENU hMenu, HINSTANCE hInst, int cmdShow) {
    this->optr->xwnd_err = XWINDOW_WINDOW_ALREADY_CREATED;  /* pre-set error to save a few code lines */

    /* not really neccessary, GetLastError would return ERROR_MOD_NOT_FOUND otherwise */
    if (this->optr->window != NULL)
        return;

    /* something to do when this->classAtom hasn't been set yet */
    if (this->optr->classAtom == 0) {
        /* try to register a standard window class or fail */
        if (this->registerWindowClass (hInst) == 0) {
            this->optr->xwnd_err = XWINDOW_FAILURE; /* so it failed */

            /* okay, last chance ... but only when the class has been registered already */
            int lastError = GetLastError ();
            if (lastError == ERROR_CLASS_ALREADY_EXISTS) {
                this->optr->xwnd_err = XWINDOW_WNDCLASS_ALREADY_EXISTS;

                /* find window handle for retrieving ATOM */
                HWND tmpWnd = FindWindow (this->optr->className, NULL);
                if (tmpWnd == NULL)
                    return;

                /* retrieve ATOM to set classAtom of this class instance */
                this->optr->classAtom = (ATOM) GetClassLong (tmpWnd, GCW_ATOM);
                if (this->optr->classAtom == 0)
                    return;
            }
            else    /* unexpected error */
                return;
        }
    }

    /* create the window */
    this->optr->xwnd_err = XWINDOW_CREATING_WINDOW_FAILED;  /* pre-set error ... again */
    CreateWindowEx (dwStyleEx, MAKEINTATOM (this->optr->classAtom), strTitle, dwStyle, posX, posY, width, height, hWndParent, hMenu, hInst, this);
    if (this->optr->window != NULL) {       /* this->window should be successfully filled by magic */
        ShowWindow (this->optr->window, cmdShow);
        UpdateWindow (this->optr->window);

        this->optr->xwnd_err = XWINDOW_SUCCESS; /* seems to be successful ... */
    }
    /* ... otherwise end with error set */
}

void XoursWindow::createWindow (wchar_t* strTitle, HINSTANCE hInst, int cmdShow) {
    this->createWindow (NULL, strTitle, WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, NULL, NULL, hInst, cmdShow);
}

void XoursWindow::createWindow (wchar_t* strTitle, int posX, int posY, int width, int height, HINSTANCE hInst, int cmdShow) {
    this->createWindow (NULL, strTitle, WS_OVERLAPPEDWINDOW, posX, posY, width, height, NULL, NULL, hInst, cmdShow);
}

int XoursWindow::destroyWindow () {
    return 0;
}

void XoursWindow::setAdditionalData (void* data) {
    this->optr->windowData = data;
}

void* XoursWindow::getAdditionalData () {
    return this->optr->windowData;
}

HWND XoursWindow::getWindowHandle () {
    return this->optr->window;
}

int XoursWindow::getLastError () {
    return this->optr->xwnd_err;
}


LRESULT CALLBACK XoursWindow::OpaquePtr::initialProc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
    /* prepare magic when non-client area is going to be created */
    if (msg == WM_NCCREATE) {
        /* create param should be filled with this-pointer */
        LPCREATESTRUCT lpcs = (LPCREATESTRUCT) lParam;
        void * lpCreateParam = lpcs->lpCreateParams;
        /* get class object .. */
        XoursWindow* xinstance = (XoursWindow*) (lpCreateParam);
        if (xinstance->optr->window != 0)       /* ... or fail */
            return 0;

        /* fill window handle (magic moment) */
        xinstance->optr->window = hwnd;

        /* set instance as user data of the window */
        SetWindowLongPtr (hwnd, GWLP_USERDATA, (LONG_PTR) xinstance);
        /* subclass window to allow generic event handling */
        SetWindowSubclass (hwnd, XoursWindow::OpaquePtr::messageForwardingProc, xinstance->optr->objectId, 0);

        /* call the event handler */
        return xinstance->optr->eventProc (xinstance, msg, wParam, lParam);
    }

    /* other things than creation of non-client area is none of our buisness at this point, but ... */
    /* we call an ugly undocumented hack to allow message handling before CreateWindow returns */
    return XoursWindow::OpaquePtr::preCreationEventHandling (hwnd, msg, wParam, lParam);    /* only called until subclassing was successful */
}

LRESULT CALLBACK XoursWindow::OpaquePtr::messageForwardingProc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam, UINT_PTR uIdSubclass, DWORD_PTR dwRefData) {
    /* prepare for message forwarding and get associated object */
    LONG_PTR lpUserData = GetWindowLongPtr (hwnd, GWLP_USERDATA);
    XoursWindow* xinstance = (XoursWindow*) lpUserData;
    if (xinstance == NULL || hwnd != xinstance->optr->window)       /* check if retrieved object is valid and verify the window handle */
        return -1;

    /* call the event handler */
    return xinstance->optr->eventProc (xinstance, msg, wParam, lParam);
}

LRESULT CALLBACK XoursWindow::OpaquePtr::eventProc (XoursWindow *ownerInstancePtr, UINT msg, WPARAM wParam, LPARAM lParam) {
    /* let's handle our events */
    switch (msg)
    {
        case WM_CREATE: {
            /* when window is going to be created we have to retrieve the object ... */
            LPCREATESTRUCT lpcs = (LPCREATESTRUCT) lParam;
            void* lpCreateParam = lpcs->lpCreateParams;
            XoursWindow* xinstance = (XoursWindow*) lpCreateParam;

            /* ... and check for sanity */
            if (xinstance != ownerInstancePtr)
                return -1;

            if (ownerInstancePtr->onCreate != NULL)
                return ownerInstancePtr->onCreate (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_PAINT: {
            if (ownerInstancePtr->onPaint != NULL)
                return ownerInstancePtr->onPaint (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_ERASEBKGND: {
            if (ownerInstancePtr->onEraseBkgnd != NULL)
                return ownerInstancePtr->onEraseBkgnd (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_SIZE: {
            if (ownerInstancePtr->onSize != NULL)
                return ownerInstancePtr->onSize (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_MOVE: {
            if (ownerInstancePtr->onMove != NULL)
                return ownerInstancePtr->onMove (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_GETMINMAXINFO: {
            if (ownerInstancePtr->onGetMinMaxInfo != NULL)
                return ownerInstancePtr->onGetMinMaxInfo (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_COMMAND: {
            if (ownerInstancePtr->onCommand != NULL)
                return ownerInstancePtr->onCommand (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_NOTIFY: {
            if (ownerInstancePtr->onNotify != NULL)
                return ownerInstancePtr->onNotify (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_TIMER: {
            if (ownerInstancePtr->onTimer != NULL)
                return ownerInstancePtr->onTimer (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_CLOSE: {
            if (ownerInstancePtr->onClose != NULL)
                return ownerInstancePtr->onClose (this->window, msg, wParam, lParam);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_DESTROY: {
            if (ownerInstancePtr->onDestroy != NULL)
                return ownerInstancePtr->onDestroy (this->window, msg, wParam, lParam);
            else
                PostQuitMessage (0);

            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;

        case WM_QUIT: {
            if (ownerInstancePtr->onQuit != NULL)           /* call onQuit */
                ownerInstancePtr->onQuit (this->window, msg, wParam, lParam);
            else if (ownerInstancePtr->eventHandler != NULL)        /* when onQuit is unset, try to call eventHandling */
                ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);

            /* it's pretty important to remove the subclass properly */
            RemoveWindowSubclass (this->window, XoursWindow::OpaquePtr::messageForwardingProc, this->objectId);
        } break;

        default: {
            return ownerInstancePtr->eventHandler (this->window, msg, wParam, lParam);
        } break;
    }
    return 0;
}

LRESULT CALLBACK XoursWindow::OpaquePtr::eventHandling (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
    /* standard event handling means calling DefWindowProc */
    return DefWindowProc (hwnd, msg, wParam, lParam);
}

main.cpp (example)

#include <Windows.h>
#include "XoursWindow/header/XoursWindow.h"

LRESULT CALLBACK owndestroy (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
    ShowWindow (hwnd, SW_HIDE);
    return 0;
}

LRESULT CALLBACK ownmove (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
    ShowWindow (hwnd, SW_HIDE);
    Sleep (100);
    ShowWindow (hwnd, SW_SHOW);
    return 0;
}

LRESULT CALLBACK ownclose (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
    if (MessageBox (NULL, L"Are you sure?", L"Last chance", MB_YESNO) == IDNO)
        return 0;
    else
        return DefWindowProc (hwnd, msg, wParam, lParam);
}

LRESULT CALLBACK events (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
    if (msg == WM_CREATE)
        MessageBox (NULL, L"eventhandling", L"", MB_OK);

    return DefWindowProc (hwnd, msg, wParam, lParam);
}

int WINAPI WinMain (HINSTANCE hThisInstance, HINSTANCE hPrevInstance, LPSTR lpszArgument, int nCmdShow)
{
    XoursWindow xwnd[4];
    wchar_t buf[50];

    lstrcpy (buf, L"XoursWindow Standard Window Class 1\0");
    xwnd[0].eventHandler = events;
    xwnd[0].createWindow (WS_EX_TOOLWINDOW, buf, WS_OVERLAPPEDWINDOW, 300, 800, 500, 240, NULL, NULL, hThisInstance, nCmdShow);

    lstrcpy (buf, L"XoursWindow Standard Window Class 2\0");
    xwnd[1].onDestroy = owndestroy;
    xwnd[1].createWindow (buf, hThisInstance, nCmdShow);

    lstrcpy (buf, L"XoursWindow Standard Window Class 3\0");
    xwnd[2].registerWindowClass (L"CustomClass\0", hThisInstance);
    xwnd[2].onClose = ownclose;
    xwnd[2].createWindow (buf, 0, 0, 500, 300, hThisInstance, nCmdShow);

    xwnd[3] = xwnd[2];
    xwnd[3].onMove = ownmove;

    MSG messages;
    while (GetMessage (&messages, NULL, 0, 0))
    {
        TranslateMessage (&messages);
        DispatchMessage (&messages);
    }

    return messages.wParam;
}

The idea was to create an event-based wrapper (quiet normal I think). To design the subclassing for fulfilling the desired mode of operation (create own events and assign them via func pointers) was the biggest "problem". There is now the possibility to assign special events (e.g. onCreate, onMove, onClose, etc.) or to set a main event handler (eventHandler). When both are set the special events have priority (eventHandler is ignored). Still missing are things like changing window size and style or include guards, but that's only a small part so I skipped them for the moment.

I've added some features like inclusion guards, more possibilities to create a window (setting size, style, etc.) and I'd put the private members into an opaque class (called OpaquePtr, only to ease maintaining when this class is used as a part of a DLL). Some bugfixing is included (e.g. behavior on onDestroy). I also added a copy constructor and a copy operator where the original object is unset when copying was successful (the current instance is assigned to the window so only one instance can be valid and the other have to be unset to avoid trouble).

\$\endgroup\$
5
\$\begingroup\$

My first question looking at this is, as a developer, what would be the advantages for me to use your window class over just dealing directly with Windows? It seems like I'd still have to do most of the same work as if I didn't use the class.

The way one would use this class appears to be:

  1. Create an instance of the class
  2. Optionally call registerClass (but not the Windows version - your class's version)
  3. Optionally set any callback pointers for my event processing
  4. Call the instance's createWindow() method

It seems like it would be easier if you made the createWindow() method a stand-alone factory function. And it would be best if your constructors allocated all the resources they need at once (or are passed in the resources they need). As it stands, your constructor may succeed at allocating the optr method, but later fail to actually allocate the HWND. If you had a factory function that allocated the window, then passed it in to the constructor, you wouldn't ever be in a position where an XoursWindow was partially constructed.

And rather than having a bunch of callback function pointers, your base class could have basic implementations of whatever the callback functions do and subclasses could override them if they need specialized functionality. (There are other better ways to handle this work, but that's beyond the scope of this answer.)

In addition to the above, I'd think hard about names. You have a struct named OpaquePtr. This name tells you nothing about it and it's wrong, to boot. You don't need to tell the reader of your code that an opaque piece of data is opaque. And you've named your struct a pointer. It's not a pointer; that shouldn't be in the name.

Your example callback functions all start with own, but you're setting member variables that start with on. I don't know if that's just trying to be cute or if it's actually a typo. It leaves open the possibility of errors due to misspellings (especially with automatic code completion!).

\$\endgroup\$
  • \$\begingroup\$ Beginning with your last remark: It wasn't a typo at all. I just made up an example and didn't take care about meaningful names. The naming of OpaquePtr based on a similiar discussion I had a few days ago which ended up in the conclusion that it doesn't matter how to name the pImpl due to the fact that the struct is related to the class only (so other names wouldn't help either). But a more meaningful name would increase the maintainability. Your remarks about implementing createWindow as factory function is very useful and I'll consider this for next improvements. \$\endgroup\$ – xourscode Oct 27 '15 at 16:03
  • \$\begingroup\$ By the way, you can swap the third with the fourth step. In this case you have to update/invalidate/etc. the window under certain circumstances (e.g. to apply changes for onPaint or onCreate). Furthermore you are allowed to pass a pointer to a WNDCLASSEX-object when calling registerWindowClass () to set up the window class with personalized data (except for lpfnWndProc and cbWndExtra). But I think that's not what you wanted to point out. \$\endgroup\$ – xourscode Oct 27 '15 at 16:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.