Stewart, Robert wrote:
Ilya Bobir wrote:
Here is a real life sample I was able to find using Google Code search: http://tinyurl.com/mlc6zo We are trying to load a cursor. There are 3 different points were it can fail. And we want to return a default cursor in case of any failure.
That's a horrible example. The loop is used to jump to cleanup code that should be handled using RAII (a sentry would work nicely).
Your answer reminded me talks on why closures should not be added to Java. There is a point that as Java has anonymous classes there really is no use for closures as one can emulate a closure with an anonymous class. While I do think that RAII is a very good approach it just does not do well in case you have to deal with a C style API. Windows API is mostly C style. As well as the old DirectX API. If all APIs were C++ and were codded to support RAII and exceptions programming in C++ would be a lot easier but it is not the case. Some of C++ programmers still have to deal with C APIs and wrapping everything to support RAII is not always the most cost effective approach.
Another context were this kind of logic may appear are parsers. Here is another real life sample: http://tinyurl.com/ljq5n9 I do not think that "cutting out" the "do { ... } while (false);" part into a separate function would add readability.
It would work nicely to create a separate function. The subfunction can return early with a flag that indicates failure and the main function can assign to status and return 0.0. Which is clearer would depend upon the reader, I suppose.
This way you may end up with a lot of functions that are named very similarly and call each other doing only very small pieces of work. While generally it seems to be a good thing when your functions are short when they became too short, because you are blindly adhering to some coding practice, the code looses readability. I think this is a good example were creating a separate subfunction will not make code more readable. I have seen some prominent examples in the Windows source of a code that is hard to understand because a simple thing is performed by a dozen of functions that call each other. It is hard to understand what a separate function does as the code is very tightly coupled but because of some coding convention it is split into different functions. I can not give you a direct link as the Windows source does not seems to be available online, but here is a stack trace of a Windows Shell call: shell32.dll!AicpMsgWaitForCompletion() + 0x36 shell32.dll!AicpAsyncFinishCall() + 0x2c shell32.dll!AicLaunchAdminProcess() + 0x2ee shell32.dll!_SHCreateProcess() + 0x59d0 shell32.dll!CExecuteApplication::_CreateProcess() + 0xac shell32.dll!CExecuteApplication::_TryCreateProcess() + 0x2e shell32.dll!CExecuteApplication::_DoApplication() + 0x3c shell32.dll!CExecuteApplication::Execute() + 0x33 shell32.dll!CExecuteAssociation::_DoCommand() + 0x5b shell32.dll!CExecuteAssociation::_TryApplication() + 0x32 shell32.dll!CExecuteAssociation::Execute() + 0x30 shell32.dll!CShellExecute::_ExecuteAssoc() + 0x82 shell32.dll!CShellExecute::_DoExecute() + 0x4c shell32.dll!CShellExecute::s_ExecuteThreadProc() + 0x25 shlwapi.dll!WrapperThreadProc() + 0x98 kernel32.dll!@BaseThreadInitThunk@12() + 0x12 ntdll.dll!__RtlUserThreadStart@8() + 0x27 The call is supposed to start a process. As you can see method names are very similar and, I believe, unless you are familiar with the code it is not obvious what each function does and unless you have a stack trace it is not even obvious the order in which the functions are called. This stack trace is a quote from this article: http://www.codeproject.com/KB/vista-security/UAC__The_Definitive_Guide.aspx
The point is that one *can* always rewrite the code a different way. Perhaps the "breakable" approach makes certain code clearer, but it seems to apply to code that is already not exception safe and very C-like.
Totally agreed. Note that not all the code out there is a proper C++ style and there are billions lines of code already written.
Cretan languages support this "breakable" construct natively. OvermindDL1 wrote that D have something similar and it was designed quite recently. I can add that Perl have direct support for this kind of control flow. In Perl it looks like this:
That other languages do something doesn't mean it is a good idea, nor does its being in Perl mean it shouldn't be in C++. ;)
The point it that the breakable is a control flow construct already "accepted" by some languages. And as such it can be treated as a control flow pattern. Not as common as "for" but still a control flow pattern, not a "bad coding style". -- Ilya Bobir