patterncppMinor
Window Creation Wrapper Class
Viewed 0 times
windowwrappercreationclass
Problem
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
XoursWindow.cpp:
```
#include "header/XoursWindow.h"
#include
#include
enum XWINDOW_ERROR {
XWINDOW_SUCCESS = 1,
XWINDOW_FAILURE,
/* registerWindowClass
*/ // ERRORS
XWINDOW_WNDCLASS_ATOM_ALREADY_SET,
/* setWindowClassAtom
*/ // ERRORS
XWINDOW_SET_WNDCLASSATOM_FAILED,
/* createWindow
*/ // WARNINGS
XWIND
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
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_HXoursWindow.cpp:
```
#include "header/XoursWindow.h"
#include
#include
enum XWINDOW_ERROR {
XWINDOW_SUCCESS = 1,
XWINDOW_FAILURE,
/* registerWindowClass
*/ // ERRORS
XWINDOW_WNDCLASS_ATOM_ALREADY_SET,
/* setWindowClassAtom
*/ // ERRORS
XWINDOW_SET_WNDCLASSATOM_FAILED,
/* createWindow
*/ // WARNINGS
XWIND
Solution
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:
It seems like it would be easier if you made the
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
Your example callback functions all start with
The way one would use this class appears to be:
- Create an instance of the class
- Optionally call registerClass (but not the Windows version - your class's version)
- Optionally set any callback pointers for my event processing
- 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!).Context
StackExchange Code Review Q#108455, answer score: 5
Revisions (0)
No revisions yet.