STOP DOING THIS:
[code language=”csharp”]
void ButtonClicked()
{
try
{
SaveEverything();
SubmitMonetaryTransaction();
SendConfirmationEmail();
}
catch
{
// If anything goes wrong, do nothing.
// Don’t log anything.
// Don’t output anything.
// Suppress any sign of the problem.
// Just continue as if everything’s completely okay.
// Also, don’t actually write a comment here.
}
}
[/code]
Let me explain why this is wrong.
A normal user is going to click your button. The user won’t see any kind of error message, which is maybe what you intended, but on the other hand, the button will seem to do nothing. The user will have no way of knowing whether she did anything wrong. The user will try it several times out of desperation. Then the user will attempt to contact you… No, actually that part may never happen. Then the user will close your app and go to bed because there is no conceivable way to determine what’s wrong or even if there is something wrong, and it’s literally too much trouble to ask. You’ll never know there’s a problem until nine months later when the user has to call you about a completely unrelated matter, and by the way, OH YEAH THE APP IS COMPLETELY BROKEN AND YOUR COMPANY SUCKS AND NOW THAT I THINK ABOUT IT I’M SWITCHING TO SOMEONE ELSE RIGHT NOW GOODBYE.
I’m not making this up.
Furthermore, at some point I’ll be summoned to fix your broken app. I may be forced to put proper exception handling in place, and correcting your atrocity may end up revealing dozens of types of errors happening all the time THAT NOBODY EVER KNEW ABOUT BEFORE. Then I’ll have to explain to the boss that Feature X has actually been silently broken for the last five years and maybe never could have possibly worked in the first place even though it’s a hook he’s been pitching this whole time.
My favorite boss reaction is utter denial, although the screaming reaction is funny, too.
I’m still not making this up.
So… Was it worth it?
If you’re no longer around when this all comes out, then… I guess you win! Cha-ching! But seriously, there is no nice way to tell you what you are.
I don’t care what made you think this was a good idea. And if it was Microsoft, well holy smokes, that’s even more reason you should have known better. STOP DOING THIS. It’s a crime against humanity.
* * *
Oh yeah, I meant for this to be constructive. Right.
There are two cases in which you should catch an exception. Two. THAT’S IT.
- You’re writing a global error handler so that if something truly unexpected happens anywhere in the whole application (except just before the handler is put in place, of course), it’ll at least get communicated to the user in an intentional way and/or logged for future review.
- You’re handling a special case in which you know what the exception is and why it’s being thrown and exactly what you’re going to do about it deliberately. This means you’re catching as narrow a class of exception as possible. And, especially when the correct reaction really is to do nothing, you put a comment there explaining the intent. I don’t care how Pythonic it was. The comment is necessary because an empty catch block is stupidly wrong so frequently that I need you to show me you know in your case it’s not.
* * *
This post was brought to you by Strong Bad. RIP Strong Bad 1996-2010.
