Wednesday, July 6, 2011

Bad Code

Unfortunately this looks familiar:

try {
DoProcessReceipts();
}
catch (Exception) { }

A C++ variant of this that drove me absolutely nuts was:

catch( ... )
{
// Do nothing
}


This sort of weak programming gets the code past QA but invariably turns in to a cluster-frack of debugging later on. My experience debugging is that it almost always becomes more difficult to track down a bug when the code intentionally swallows related errors.

So, what's the solution? Well, you could make an empty control block throw a warning. That might help some people be more responsible but I suspect a greater portion would simply do something worse like this:

catch( ... )
{
int x = 0;
}

At the end of the day I want to build stable tools that help people get stuff done. Most developers, even those in the field for money instead of passion, want to do a good job. But "doing it right" must be balanced against "getting it done" and there's the rub.

Not every exception needs to be captured and handled in the frame where it occurs but sometimes it does make sense. Look at the big picture in the code but also look at the business situation. Catch the error in the appropriate frame, catch the appropriate types, and record them every. single. time. Given the inevitability of bogus data from bad error handling on a long running process, it may better over the long run to simply crash and use a dump on the ab-end to fix the bug instead of leaving a process with fubar state running. It all depends on the business needs and technical situation. Please take a minute to think it through. It is better for everyone involved to talk it through with a colleague and build a better program in the short run. If you don't, I promise you that someone-- maybe you-- will be stuck picking back through that code trying to figure out what keeps randomly failing.

No comments: