Let Your Code Fail Early

To the list of disturbing practices I nominate this:

if ([some critical resource] != null)
    [do something with this critical resource];

The critical resource above could be an essential configuration setting, a file, a control, etc. Its absense should lead to an exception so that you, the developer, can figure out this anomalous flow early, before the product ships.

For example, during a recent code review, I found a chunk of code responsible for validation. There was an explicit check for a validator, and if one wasn’t found then, well, validation didn’t happen. In this application, if a validator goes missing, it’s an abnormal situation which we should catch in development. Obfuscating it with a null check only leads to trouble.

The twin brother of evil null checks is a swallowed exception:

try
{
  // some important logic
}
catch (Exception ex)
{
}

If you’re absolutely, positively sure it’s Ok to swallow an exception like this, fine. But this happens very, very rarely.

Catching general Exceptions is almost always a bad thing. But it's even worse when the developer didn't document why all exceptions should be caught. While unlikely, the developer that wrote the code could have a legitimate reason to do so, but it's impossible for us to figure out if and why it's correct. The guidelines I wrote for my company state that a developer should explicitly document why a exception is not rethrown in a catch statement. The absence of this documentation, gives another developer full permission to remove the catch completely.

Great point about documentation, Steven. The rationale for swallowing an exception should be documented... or the catch clause must go.

@steven - I have been contracted out for work with several companies that require coders documentation. Unfortunately, our mindset (or better stated a typical mindset) is that by the time you have worked out all of the kinks, you are late for getting to the next project. So good documentation happens about as often as swallowing an exception. If it has worked, then that is great and it really is a good idea. But if not, then I have seen companies offer incentives for "clean coding". Similar concept as commission for sales.

The only time I ever bypassed the exception purposely was when I was in school or I simply wanted to get a project over with and just wanted the code to work. Never kept it in the code though.

I ran a trial of ReSharper (JetBrains) and it implied that there was something wrong if a null was possible anywhere for any reason (well, maybe that's a very slight exaggeration). Click on the little orange tag and it'll suggest a "if ([some critical resource] != null)". Hrump.

Yep, ReSharper does panic like that sometimes.