There's an ongoing religious war among C++ programmers about the right way to handle errors:
- Exceptions: Elegant, but not fully portable. Improves separation from functional code and error-handling code. Some performance concerns.
- Return codes: Lightweight. Portable. Really easy to ignore.
I read an interesting couple of articles today that propose an interesting compromise: return values that you can't ignore. The articles aren't new, but they were new to me; maybe you'll find them interesting too.
The refinement is better? I don't agree... shouldn't
ReplyDeleteif(fallibleFunction()) { ... }
be considered good practice? Having to explicitly instantiate a holder object for the return value is a weakness as it makes the code longer and less expressive (although I would encourage use of if(0 != fallibleFunction()), personally).
Certainly the "problem" example code introduced by the follow up author does demonstrate possible process termination. However, it is also an example of bad practice! The first function's error code IS being ignored... who is the caller to judge that calling fallibleMethod_2() without first checking for success in fallibleMethod_1() is legitimate? If it was, a joint method should have been exposed by the API that returned only one ErrorCode<> for the compound operation.
So the problem example is wrong because it terminates rather than merely throwing, but here again the error lies with the example use: the author's insistence on catching the return values as ErrorCode<int> s instead of as regular int s is bad practice. Never do this, and the code will work as desired (although he would still be foiling the bad-practice detection system). You can even write it more concisely with NO local variables AND without his error as:
if((0 != this->fm_1()) && (0 != this->fm_2()))
carryOn();
So, the extension to the ErrorCode<> class that's needed is really only something to force its instantiations to be temporary-only. This may be impossible without compiler help: an approach that counts constructions may be foiled by the fact that (I believe) the number of constructions involved in passing back an object by value is implementation- and optimization-level-dependent. However, strategic use of the preprocessor could probably do the trick since all temporaries (ie the only legitimate instantiations) are invisible to it. Something like
#define ErrorCode COMPILE_TIME_ASSERT(false)
would prevent non-temporary instantiations from making it to the parser if things could be arranged so that it took effect only after all legitimate namings of the ErrorCode<> class had occurred... as far as I can tell it need only be named in the return type of fallible method prototypes and implementations.
Or am I missing something?
I think it's pretty outrageous that the C++ runtime will terminate processes for having two exceptions pending at once, but that's a topic for another time. :-)
ReplyDeleteI'm uncomfortable with the refinement too. The missing conversion to the "real" error code type destroys the transparency of the idiom, which is (in my view) one of its primary benefits.
I agree that
if (fallibleFunction()) { ... }
should be considered good practice. It nests the real logic of the application inside conditionals, but that's a problem with error codes in general, not this pattern specifically.
I'm not totally ready to give up on the refinement altogether -- maybe there's a further sub-refinement that could save it. In particular, playing devil's advocate:
* Avoiding process termination in the case of a mishandled error is no small thing. When possible, fail fast -- but give callers a chance to degrade gracefully. That's especially important when you think about integrating independently-developed components, not just writing whole standalone applications.
* Re: the "problem" code in the follow-up article: it's true that postponing the error check is suspect. But depending on the degree of coupling between the two calls, it could also be correct. In fact, the less the coupling, the more likely deferral is to be correct, and the less appropriate a combined wrapper method would be. Especially in a language like C++, I think you've got to give the programmer the benefit of the doubt. Though your combined 'if' proposal isn't bad.
I think you may be onto something with your temporary-only comment at the end. The temporary-related Idea in the first article suggests an interesting approach to that problem, although it's not quite as transparent and fool-proof as I would like.