Hell, I am furious! I come to work on a bloody Saturday, to review some code so that we can release our new product on time and I stumble twice on one of the most basic coding crimes when it comes to error handling, each done by a different developer.
I know it's natural to make errors, that's why I do the code review in the first place right? Absolutely! The reason that I am furious is that one of the two instances of this crime is long ago commited to our product (an SDK) and thus I cannot correct it... because... I will break backwards compatibility. Damn it!
The other one is not released yet, so it's just a human error; we'll just have to update about 70 calls to some internal function and we are done.
So what exactly is this crime I am talking about?
Simply put, it is a CAPITAL OFFENSE to convert an error code to a boolean Success_or_Failure flag.
A variation of this crime is to convert a rich error code into something like E_FAIL (in the COM world).
In real life, most programs cannot do much in the face of errors, other than (a) not crash (b) let the user know what went wrong.
In real life, when something goes wrong the user will immediately contact technical support. So imagine being the tech personnel, having a frustrated and often angry user telling you "Hey, I tried to do such and such and it fails with the error E_FAIL (Unspecified Error). What the hell is going on?".
Exactly my point! What the hell is going on???? How is our tech support team supposed to provide decent technical support if we don't know exactly what error occurred?
Most developers I have encountered in my career so far don't have the proper understanding and respect for error handling code. They want to write the REAL code not the error handling crap (or heavens forbid... the documentation).
They don't understand or just don't want to realize that in solid systems the error handling code can be up to 50% of the code. Yeap, that's FIFTY percent. And if you don't write it properly then you are NOT doing 50% of your job. Think a little bit about it.
Moreover, in most cases you don't write the error handling code for your end user, you write it FOR YOUR OWN SHAKE, to make YOUR LIFE easier when something goes wrong to the user. Because as sure as death and taxes are, things going wrong to the user is a certainty.
OK, now that I've had my fair share of whining I am not furious any more... Back to business :-)
Have fun!
I totally share your anger! I faced exactly the same problem a few days ago while doing some C# code review myself (returning enumeration instead of the exception itself). I suppose you are talking about C++?
Fortunately in .NET world plug-ins like Resharper display warnings (which you can upgrade to errors) when the exception variable in a try/catch block is not handled properly or not handled at all. Unfortunately the developer in this case completely ignored this warning! This is another common problem which I would name 'Warning handling crime'!
I have to disagree thought that to do proper error handling you need to double your code. I don't know about C++ but in .NET world in most cases not writing exception handling (I know that I'm making) works fine. Only in specific cases, such as when you want to translate an exception into a more explicit one (of course there are other cases also). An in 99% of these cases you need to put the original exception as InnerException of the new more explicit one. If you don't then it could be as tragic as using the return value!
ps. I'm doing the convention here that error and exception are equal which is not completely true. But in the context we are talking about I guess its ok.
Posted by: Dimitris Papadimitriou | July 29, 2008 at 02:45 PM