Is This Bad Practice

[Erik Wiggins]

I'm still new to programming with c++ and I'm still working on my own style for programming. I've found a technique that works real well for me however I'm not sure it's a good practice as I've never seen anyone use anything like it in this forum or in any of the articals/tutorials I've read. Before I start using it alot I figure I better run it by you guys first. This is what I've done.

3 classses


class wnd
{
public:
BOOL Show(int nCmdShow);
void Create();
CREATESTRUCT cs;
wnd();
wnd(HINSTANCE hinstance);
virtual ~wnd();
HWND hWnd;
};

class cls
{
public:
ATOM Register();
WNDCLASSEX wcx;
cls();
virtual ~cls();
}

class wndcls :
public wnd,
public cls
{
public:
wndcls();
virtual ~wndcls();

};


Then in order to create a window I derive a class from one of those three classes and put the WNDPROC in the cpp file of the derived class.


//header
class wndclsFrame : public wndcls
{
public:
wndclsFrame(HINSTANCE hInst);
virtual ~wndclsFrame();

};

//cpp
#include "wndclsFrame.h"
#include "resource.h"

LRESULT CALLBACK MainWndProc(HWND,UINT,WPARAM,LPARAM);

wndclsFrame::wndclsFrame(HINSTANCE hInst)
{
wcx.cbSize = sizeof(wcx);
wcx.style = CS_HREDRAW | CS_VREDRAW;
wcx.cbClsExtra = 0;
wcx.cbWndExtra = 0;
wcx.hIcon = LoadIcon(NULL, MAKEINTRESOURCE(IDI_ICON1));
wcx.hCursor = LoadCursor(NULL,IDC_ARROW);
wcx.hbrBackground = GetStockObject(WHITE_BRUSH);
wcx.lpszMenuName = MAKEINTRESOURCE(IDR_MENU1);
wcx.hIconSm = NULL;
wcx.lpfnWndProc = MainWndProc;
wcx.lpszClassName = "FrameWndClass";
wcx.hInstance = hInst;

Register();

cs.lpCreateParams = NULL;
cs.hMenu = (HMENU) NULL;
cs.hwndParent = (HWND) NULL;
cs.cy = CW_USEDEFAULT;
cs.cx = CW_USEDEFAULT;
cs.y = CW_USEDEFAULT;
cs.x = CW_USEDEFAULT;
cs.style = WS_OVERLAPPEDWINDOW;
cs.dwExStyle = WS_EX_APPWINDOW;
cs.hInstance = hInst;
cs.lpszName = "Sample";
cs.lpszClass = "FrameWndClass";

Create();
Show(SW_SHOWNORMAL);
}

wndclsFrame::~wndclsFrame()
{

}

LRESULT CALLBACK MainWndProc(HWND hwnd,UINT uMsg,WPARAM wParam,LPARAM lParam)
{

switch (uMsg)
{
case WM_CREATE:

break;

case WM_PAINT:
// Paint the window's client area.
break;

case WM_SIZE:
// Set the size and position of the window.
break;

case WM_DESTROY:
PostQuitMessage(0);
break;

default:
return DefWindowProc(hwnd,uMsg,wParam,lParam);
}
return 0;
}


That way I can keep my WinMain really breif.


#include <windows.h>
#include "resource.h"
#include "wndclsFrame.h"

MSG msg;

// Application entry point.

int WINAPI WinMain(HINSTANCE hinstance, HINSTANCE hPrevInstance,
LPSTR lpCmdLine, int nCmdShow)
{

wndclsFrame Frame(hinstance);

while(GetMessage(&msg, NULL, 0, 0))
{
DispatchMessage(&msg);
}
return msg.wParam;
UNREFERENCED_PARAMETER(lpCmdLine);
}


It also make it real simple for me to keep trake of my windows as all the information for the window is in it's own nice little file. Instead of having a bunch of Init functions in my App.cpp file containing WinMain.

[Paul McKenzie]

There is nothing wrong with what you are doing. However, I would refactor the common window creation steps into the base class, cls (I would also call it a different name)

For example, what if you now want to create another window with a different class, call it wndclass2? You will see that you will need 99% of everything that wndclass does in the constructor, with the only differences being the class name, title, etc.

Instead of copying and pasting the entire wndclass constructor and replacing the title, place the common thing that all windows need to do when initializing, and put that in the base class.

Also, the WndProc can also be more object-oriented. Have a map of HWND's to pointers to cls objects. When you create a window, associate the HWND with the new class object. When the window procedure is called, use the passed in HWND to lookup the corresponding clas, and call the cls WindProc function.

Here is an example:

#include <map>
class BaseWnd;
typedef <HWND, BasWnd* > WndMap;

class BaseWnd
{
public:
static LRESULT CALLBACK WndProc( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lParam);

virtual LRESULT DoWndProc(HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lParam) = 0;

void AddToMap( HWND hWnd, BaseWnd *pWnd )
{
MyWndMap[hWnd] = pWnd;
}
private:
static WndMap MyWndMap;
};

//...
LRESULT BaseWnd::WndProc(HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lParam)
{
BaseWnd *pWnd;
WndMap::iterator it = MyWndMap.find(hWnd);
if ( it != MyWndMap.end( ) )
{
pWnd = it->second;
return pWnd->DoWndProc( hWnd, nMsg, wParam, lParam);
}
return DefWindowProc(hWnd, nMsg, wParam, lParam);
}

// In only one CPP file, do this:
WndMap BaseWnd::MyWndMap;

And then when you create the window, add it to the map. What this does is that you can now derive from BaseWnd, and each one has its own "WndProc". The base WndProc calls the derived window procedure. Plus, the derived WndProc is not a static member -- it is now a non-static class member, and you have access to non-static member variables, etc.

I didn't write all the code, but you should get the idea.

Regards,

Paul McKenzie

[laasunde]

Paul McKenzie: Shouldn't the typedef in your code be like this ?


typedef std::map<HWND, BaseWnd* > WndMap;


Just wondering like..

[NoHero]

Paul McKenzie: Shouldn't the typedef in your code be like this ?


typedef std::map<HWND, BaseWnd* > WndMap;


Just wondering like..

You can use the typedef or not. As you wish ...

[Andreas Masur]

You can use the typedef or not. As you wish ...
He was rather pointing out the missing 'std::map'... :cool: