Ilya Bobir wrote:
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).
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.
I do whatever I can to avoid dealing with C APIs.
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.
I much prefer RAII even when not dealing with exceptions. It makes the code clearer.
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.
That can happen, but its rare.
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.
Without details on how complex those functions are, neither of us can comment on this example. If the functions in that stack trace are non-trivial, if they are reused, etc., the factorization was a good idea. If they are tiny functions that take lots of arguments, then the factorization was a bad idea.
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.
When maintaining code, one often has the choice of inserting something like the "breakable" approach, refactoring, adding RAII, etc.
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".
"That other languages do something doesn't mean it is a good idea." Just because it is codified in other languages as a control flow pattern doesn't mean it isn't bad coding style. We seem to be agreeing violently that the "Breakable Idiom" may make some code clearer but that there are many alternatives that bring other benefits, so I'll not say more. The question is whether Boost will accept it. The only way to really know the answer to that question is to have it reviewed. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.