Switch statement use?

There's nothing wrong with how you have it, but it's probably cleaner code to call a function so you can keep your functions a reasonable size.

He does have a break, but it's outside the braces. Makes it hard to read. Also, the indentation hurts readability.

And readability is another benefit of making a function, provided that he can think of a good function name. – Alan Hensel Apr 5 '09 at 0:09 The break was put in after Brian posted his solution. – DeadHead Apr 5 '09 at 0:14 I removed that line since it was corrected in the question.

– Brian R. Bondy Apr 5 '09 at 0:21 1 I tend to put the break after the closing brace. Dunno realy why, maybe as a "no matter what you do, don't forget to break" reminder (to myself).

– peterchen Apr 5 '09 at 7:54.

Or the C++ equivalent: ATL's CWindow class. Your window's cpp file will look really clean. – Leonardo Constantino Apr 6 '09 at 9:28.

Or start to use rule - one message = one function call. Char szFileNameMAX_PATH; HINSTANCE hInstance = GetModuleHandle(NULL); GetModuleFileName(hInstance, (LPWCH)szFileName, MAX_PATH); MessageBox(hwnd, (LPCWSTR)szFileName, L"This program is:", MB_OK | MB_ICONINFORMATION); Could you explain this strange trick with convetation (LPCWSTR)szFileName. Why you don't use array wchar_t instead casting?

- you will have big problem with long paths ( path_length > MAX_PATH / sizeof( wchar_t ) ) One reccomendation - avoid to use casts in general and C-style casts in particulary.

If you are asking if you should turn the code in the first case into a function, then yes, definitely.

Well, it would depend on how many other cases you would have. Something as small as that, I would say it is not worth it to make it a function, but if your switch statement contains more cases, it will just get ugly, especially if a lot of the cases has multiple lines like that. Putting it into a function would clean it up and make your code look nicer.

One of the most important things I'd say would be consistency. If you create a function for LBUTTONDOWN then create a function for everything. This way there is a predictable pattern for where to find stuff if it breaks.

Related to the topic at hand: I personally find an if / else if pattern to work better, as it eliminates the problem of a forgotten break: if (msg == WM_LBUTTONDOWN) { // your code here return 0; } else if (msg == WM_DESTROY) { PostQuitMessage(0); return; } else if (msg == WM_KEYDOWN) { if (wp == VK_F1) { DoSomething(); return; } } return DefWindowProc(hWnd, msg, wp, lp); It's really up to you, in the end.

IF your cases grow large though, the switch/case can (sometimes) be more efficient. I remember when I was coding a 6510 emulator and the opcode decoder started out as an IF statement (i was young). It was slow.

Using a case statement resulted in a jump table and it was, well, faster! – Adam Hawes Apr 5 '09 at 0:01 Steve Mcconnell from Code Complete tested this to be true in both ways, sometimes case statements are faster, sometimes if else constructs are faster. Sometimes there is no difference.So don't worry about it until there are performance problems, then profile, change and profile again – Emile Vrijdags Apr 7 '09 at 14:15.

I would probably declare a map and use functors for every message: typedef std::map > messageFuncs_t; messageFuncs_t messageFuncs; Then, when the window class is created, just add a new function for each message: messageFuncsWM_LBUTTONDOWN = &onMouseDownEvent; ... And then implement the message loop thus: messageFuncs_t::iterator fun = messageFuncs. Find(msg); if(fun! = messageFuncs.end()) return (*fun)(hWnd, wparam, lparam); else return DefWindowProc(hWnd, msg, wp, lp); ... Or whatever works.

Then it's easy to add new messages, and the work for each is delegated to a function. Clean, concise, and makes sense.

If you're going to go to the trouble of . Find'ing the message, there's no reason to do a second lookup (operator) to invoke it. Also, you're using pointer to function syntax with boost::function, which is unecessary (even with actual function pointers), and which I'm pretty sure won't work.

– Logan Capaldo Apr 5 '09 at 18:52 You're right. I threw this together quickly and didn't bother consulting documentation. I'll fix it.

– greyfade Apr 5 '09 at 22:17.

You are missing a break on the first case. Other than that, I would definitely put that code on a separate function.

The break is after the closing bracket. – EvilTeach Apr 4 '09 at 23:49.

It's fine, but I generally don't like mixing styles and indentations. If I needed to bracket one case I'd probably bracket them all and keep the indentation consistent. Also bb is right, you should use wchar_t array instead of char in that case.

I'm writing fairly many Win32 message crackers such as these switches. My rule of thumb is: Wiring up the behavior goes into the switch, behavior into a separate function. That usually means the switch contains the decision whether this command should be handled (e.g. Testing for sender ID), and "prettyfying" the parameters.

So in that particular case, a separate function. The original motivation is that I often end up triggering the behavior in other circumstances (e.g."when no file name has been specified and the dialog is called with the moon parameter set to full, show the SaveAs dialog immediately").

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.