Efficiency coding; a simple top-level window

I'd like to know how other programmers go about creating a simple window in Win32. If there's anything in my code that could be made more efficient, or which has been superseded, I'd love to know, thanks.

Code:

#include <windows.h>

int WINAPI WinMain(HINSTANCE hInstance,HINSTANCE hPrevInstance,LPSTR lpCmdLine,int nCmdShow);
LRESULT CALLBACK WndProc(HWND hwnd,UINT msg,WPARAM wParam,LPARAM lParam);

int WINAPI WinMain(HINSTANCE hInstance,HINSTANCE hPrevInstance,LPSTR lpCmdLine,int nCmdShow)
{
	WNDCLASSEX wcx;
	
	wcx.cbClsExtra=0;
	wcx.cbSize=sizeof(WNDCLASSEX);
	wcx.cbWndExtra=0;
	wcx.hbrBackground=(HBRUSH)(COLOR_BTNFACE+1);
	wcx.hCursor=LoadCursor(NULL,IDC_ARROW);
	wcx.hIcon=LoadIcon(NULL,IDI_APPLICATION);
	wcx.hIconSm=NULL;
	wcx.hInstance=hInstance;
	wcx.lpfnWndProc=WndProc;
	wcx.lpszClassName="clsAccess";
	wcx.lpszMenuName=NULL;
	wcx.style=0;

	if (!RegisterClassEx(&wcx))
		exit(1);

	HWND hwnd;

	hwnd=CreateWindow("clsAccess","Access Manager",
				WS_OVERLAPPEDWINDOW,
				0,0,0,0,NULL,NULL,hInstance,NULL);

	if (!hwnd)
		exit(2);

	ShowWindow(hwnd,SW_SHOW);
	UpdateWindow(hwnd);

	MSG msg;

	while (GetMessage(&msg,hwnd,0,0)>0)
	{
		TranslateMessage(&msg);
		DispatchMessage(&msg);
	}

	return msg.wParam;
}

LRESULT CALLBACK WndProc(HWND hwnd,UINT msg,WPARAM wParam,LPARAM lParam)
{
	switch (msg)
	{
	case WM_DESTROY:
		PostQuitMessage(0);
		return 0;

	case WM_CREATE:
		SetWindowPos(hwnd,NULL,
			GetSystemMetrics(SM_CXSCREEN)/4,
			GetSystemMetrics(SM_CYSCREEN)/4,
			GetSystemMetrics(SM_CXSCREEN)/2,
			GetSystemMetrics(SM_CYSCREEN)/2,
			SWP_NOOWNERZORDER);
		break;

	default:
		break;
	}

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

More efficient? If your finding that slow then I guess it's more to do with the os than your code

Though as you seem to be in C++ (with variables declared throughout the block and not at the top), why not

WNDCLASSEX wcx = {0};

Then you dont need to keep assigning NULL or zero to the relevant members of that struct.

Also, why bother calling exit() when you can use a simple return with that value you want and allow the compiler generated code to destroy the process.

None of these are real efficiency tips...you would never in a million years notice a benefit from my suggestions

It's not for speed, it's just general things that I should be doing. Looks like it's okay though. Thanks.

Warning

Because the return value can be nonzero, zero, or -1, avoid code like this:

Code:

while (GetMessage( lpMsg, hWnd, 0, 0)) 
...

The possibility of a -1 return value means that such code can lead to fatal application errors. Instead, use code like this:

Code:

BOOL bRet;

while( (bRet = GetMessage( &msg, NULL, 0, 0 )) != 0)
{ 
    if (bRet == -1)
    {
        // handle the error and possibly exit
    }
    else
    {
        TranslateMessage(&msg); 
        DispatchMessage(&msg); 
    }
}

Eibro, thanks for the tip. Now here's a tip for you: go back and read my code.

Hahahaha!

But the documentation for GetMessage() says that sucess could be anything non-zero or -1. So -5 could be success. I'd recommend using Eibro's code anyways.

Originally posted by bennyandthejets
Eibro, thanks for the tip. Now here's a tip for you: go back and read my code.

Again, here's a tip for you: Do what I suggested. That was quoted straight from MSDN.

I was indignant until I read Speedy5's addition to his/her original post. I had forgotten that non-zero included negatives too. Sorry Eibro. I haven't run into any problems in the past with that one. Has anyone?

EDIT: Being a follower of the Church of MSDN doesn't make everything you say correct. Speedy5's explanation had substance to it.