Win32API program Main with Child windows design pattern

Question

Below is the code of a development status project. I have designed a main window with 3 children. I am mainly interested in your comments on the design pattern used for drawing the child windows and the controls per child. The childwindows will be selected via the menu (show and hide). the logic of the control will be handled in the childproc.

#if defined _MSC_VER || defined __BORLANDC__
#define OEMRESOURCE
#endif
#pragma comment(linker,"\"/manifestdependency:type='win32' \
name='Microsoft.Windows.Common-Controls' version='6.0.0.0' \
processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'\"")
#include <windows.h> //include all the basics
#include <tchar.h> //string and other mapping macros
#include <iostream>
#include <string>
#include "hrmcom.h"
#include "resource.h"
typedef std::basic_string<TCHAR> ustring;
LRESULT CALLBACK WndProc(HWND, UINT, WPARAM, LPARAM);
LRESULT CALLBACK ChildProc(HWND, UINT, WPARAM, LPARAM);
inline int ErrMsg(const ustring&);
bool CreateChild(HWND);
bool CreateChildScreen(HWND);
ustring classname = TEXT("MAINWND");
ustring childname = TEXT("CHILDWND");
HWND hwndChild[3], hwndCtrl[CTRNUM];
int WINAPI WinMain(HINSTANCE hInst, HINSTANCE, LPSTR pStr, int nCmd)
{
 WNDCLASSEX wcx = { 0 }; 
 wcx.cbSize = sizeof(WNDCLASSEX);
 wcx.lpfnWndProc = WndProc; 
 wcx.hInstance = hInst; 
 wcx.hIcon = LoadIcon(hInst, MAKEINTRESOURCE(IDI_ICON1));
 wcx.hCursor = reinterpret_cast<HCURSOR>(LoadImage(0, IDC_ARROW,
 IMAGE_CURSOR, 0, 0, LR_SHARED));
 wcx.hbrBackground = reinterpret_cast<HBRUSH>(COLOR_BTNFACE + 1);
 wcx.lpszClassName = classname.c_str();
 if (!RegisterClassEx(&wcx))
 {
 ErrMsg(_T("Failed to register wnd class"));
 return -1;
 }
 wcx.lpfnWndProc = ChildProc;
 wcx.cbWndExtra = sizeof (long);
 wcx.hIcon = NULL;
 wcx.lpszClassName = childname.c_str();
 if (!RegisterClassEx(&wcx))
 {
 ErrMsg(_T("Failed to register wnd class"));
 return -1;
 }
 HMENU hMenu = LoadMenu(hInst, MAKEINTRESOURCE(IDR_MENU));
 HWND hwnd = CreateWindowEx(0, 
 classname.c_str(), 
 TEXT("Polar Loader"), 
 WS_OVERLAPPED | WS_CAPTION | WS_SYSMENU, 
 400, 200, 600, 300, 
 0, 
 hMenu, 
 hInst, 
 0); 
 if (!hwnd)
 {
 ErrMsg(TEXT("Failed to create wnd"));
 return -1;
 }
 ShowWindow(hwnd, nCmd);
 UpdateWindow(hwnd);
 MSG msg;
 while (GetMessage(&msg, 0, 0, 0)>0)
 {
 TranslateMessage(&msg);
 DispatchMessage(&msg);
 }
 return static_cast<int>(msg.wParam);
 }
 LRESULT CALLBACK WndProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
 {
 switch (uMsg)
 {
 case WM_CREATE:
 //Create the Childwindows
 if (!CreateChild(hwnd))
 {
 ErrMsg(TEXT("Failed to create Childwindows"));
 return -1;
 }
 //Create controls of the Childwindows 
 if (!CreateChildScreen(hwnd))
 {
 ErrMsg(TEXT("Failed to create Conrols of Childwindows"));
 return -1;
 }
 return 0;
 case WM_COMMAND:
 switch (LOWORD(wParam))
 {
 case IDM_Acc:
 ShowWindow(hwndChild[0], SW_SHOW);
 return 0;
 case IDM_Con:
 case IDM_Help:
 case IDM_Log:
 case IDM_Pol:
 case IDM_Trans:
 ShowWindow(hwndChild[0], SW_HIDE);
 return 0; 
 }
 case WM_DESTROY:
 PostQuitMessage(0); //signal end of application
 return 0;
 default:
 //let system deal with msg
 return DefWindowProc(hwnd, uMsg, wParam, lParam);
 }
}
LRESULT CALLBACK ChildProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
 switch (uMsg)
 { 
 case WM_CREATE:
 SetWindowLong(hwnd, 0, 0); 
 return 0;
 case WM_COMMAND:
 switch (LOWORD(wParam))
 {
 case ID_CTRL+5: 
 MessageBeep(0);
 return 0;
 }
 default:
 return DefWindowProc(hwnd, uMsg, wParam, lParam);
 }
}
inline int ErrMsg(const ustring& s)
{
 return MessageBox(0, s.c_str(), _T("ERROR"), MB_OK | MB_ICONEXCLAMATION);
}
bool CreateChild(HWND hwnd)
{
 static int x;
 for (x = 0; x < 3; x++)
 {
 hwndChild[x] = CreateWindowEx(0, childname.c_str(), NULL,
 WS_CHILDWINDOW,
 0, 0, 600, 300,
 hwnd, (HMENU)x,
 (HINSTANCE)GetWindowLong(hwnd, GWL_HINSTANCE),
 NULL);
 if (!hwndChild[x]) return FALSE;
 }
 return TRUE;
}
bool CreateChildScreen(HWND hwnd)
{
 LPCWSTR lpType[CTRNUM] = { TEXT("static"), TEXT("edit"), TEXT("edit"),TEXT("static"), TEXT("static"), TEXT("button") };
 LPCWSTR lpName[CTRNUM] = { TEXT("Log on to Gedysan Training App:"), TEXT(""), TEXT(""), TEXT("Username:"), TEXT("Password:"), TEXT("Log on") };
 DWORD dStyle[CTRNUM] = { WS_CHILD | WS_VISIBLE | ES_LEFT,
 WS_CHILD | WS_VISIBLE | ES_LEFT | WS_BORDER | ES_AUTOHSCROLL,
 WS_CHILD | WS_VISIBLE | ES_LEFT | WS_BORDER | ES_PASSWORD | ES_AUTOHSCROLL,
 WS_CHILD | WS_VISIBLE | ES_LEFT,
 WS_CHILD | WS_VISIBLE | ES_LEFT,
 WS_CHILD | WS_VISIBLE | ES_LEFT
 };
 RECT rc[CTRNUM] = { { 15, 10, 400, 25 }, { 100, 40, 150, 25 }, { 100, 70, 150, 25 }, { 15, 45, 70, 25 }, { 15, 75, 70, 25 }, { 15, 110, 70, 25 } };
 int iChildNum[CTRNUM] = { 0, 0, 0, 0, 0, 0 };
 static int x;
 for (x = 0; x < CTRNUM; x++)
 {
 RECT x1 = rc[x];
 hwndCtrl[x] = CreateWindowEx(0, lpType[x], lpName[x], dStyle[x],
 x1.left, x1.top, x1.right, x1.bottom,
 hwndChild[iChildNum[0]], (HMENU)(ID_CTRL + x),
 (HINSTANCE)GetWindowLong(hwnd, GWL_HINSTANCE), 0);
 if (!hwndChild[x]) return FALSE;
 }
 return TRUE;
}

• JaDogg: What's with the indentation ? I don't know C++ but where/what is hrmcom.h ?
• gedysan: @JaDogg: I am not sure what you mean with indentation. Can you state that question in dutch-english...? The headerfile hrmcom.h belongs to a thrd party DLL. It is used to read Polar heart devices via infrared port.
• JaDogg: Indentation problem was fixed no worries. Please mention the third party libraries you are using, In case it might help reviewers.

Answer 1

Your code is not an easy read, since it relies heavily on the Windows API. I can't go very deep into that, but will give some suggestions you can use to improve the overall code quality.

Constants that are not const:

These strings are constants, but are not marked const:

ustring classname = TEXT("MAINWND");
ustring childname = TEXT("CHILDWND");

Fix:

const ustring classname = TEXT("MAINWND");
const ustring childname = TEXT("CHILDWND");

Use unnamed namespaces:

If you must use globals, at least wrap then into an unnamed namespace to effectively make the variables local to the file:

namespace 
{
 const ustring classname = TEXT("MAINWND");
 const ustring childname = TEXT("CHILDWND");
 HWND hwndChild[3], hwndCtrl[CTRNUM];
}

Inline:

Personally, I don't like to declare a prototype to an inline function. I think it is more logical to declare and define inline functions in one place. I would suggest moving the definition of ErrMsg() to the top of the file (possibly inside the unnamed namespace) and removing the function prototype declaration.

Use of NULL:

NULL does not belong in C++. If you are in a pre-C++11 compiler, 0 would be the ideal. In moder C++, you use nullptr.

Unnecessary static variables:

For some unexplained reason, you've used a static variable for a loop counter (in two places, CreateChild() and CreateChildScreen()).

static int x;
for (x = 0; x < 3; x++)

This makes no sense. Why would you want that static if the variable is set to zero on its first reference? Static variables are used when you need to maintain state between calls of the same function. Clearly not the case here.

Use more const:

Stuff that don't get written to after construction, including local variables inside functions, should be marked as const. This eliminates undesired assignment errors.

The arrays inside CreateChildScreen() are good candidates for that.

Hardcoded constants:

Your window sizes and positions are all hardcoded inside the functions. You might consider replacing some of those with named constants.

• gedysan: Wow, Thx glampert. This is very usefull feedback. I will improve the code with your suggestions. Can you tell me if i used the right way of building up the child screens (with the function CreateChildScreen and the arrays). I have not found a good reference on how the design pattern on these look like. Thx again!
• glampert: @gedysan, sorry, I haven't worked with the WinAPI in a long time. But I would recommend that you try to make your code more object-oriented. Introduce some classes to handle the windows/screens (i.e.: a Window or Screen class).