Empty catch blocks

Question

I sometimes run into situations where I need to catch an exception if it's ever thrown but never do anything with it. In other words, an exception could occur but it doesn't matter if it does.

I recently read this article about a similar thing: http://c2.com/cgi/wiki?EmptyCatchClause

This person talks about how the comment of

// should never occur 

is a code smell and should never appear in code. They then go onto explain how the comment

// don't care if it happens

is entirely different and I run into situations like this myself. For example, when sending email I do something similar to this:

var addressCollection = new MailAddressCollection();
foreach (string address in addresses)
{
    try
    {
        addressCollection.Add(address);
    }
    catch (Exception)
    {
        // Do nothing - if an invalid email occurs continue and try to add the rest
    }
}

Now, you may think that doing this is a bad idea since you would want to return to the user and explain that one or more messages could not be sent to the recipient. But what if it's just a CC address? That's less important and you may still want to send the message anyway even if one of those addresses was invalid (possibly just a typo).

So am I right to use an empty catch block or is there a better alternative that I'm not aware of?

Solution 1

You are completely right to use an empty catch block if you really want to do nothing when a certain type of exception occurs. You could improve your example by catching only the types of exceptions which you expect to occur, and which you know are safe to ignore. By catching Exception, you could hide bugs and make it harder for yourself to debug your program.

One thing to bear in mind regarding exception handling: there is a big difference between exceptions which are used to signal an error condition external to your program, which is expected to happen at least sometimes, and exceptions which indicate a programming error. An example of the 1st would be an exception indicating that an e-mail couldn't be delivered because the connection timed out, or a file couldn't be saved because there was no disk space. An example of the 2nd would be an exception indicating that you tried to pass the wrong type of argument to a method, or that you tried to access an array element out of bounds.

For the 2nd (programming error), it would be a big mistake to just "swallow" the exception. The best thing to do is usually to log a stack trace, and then pop up an error message telling the user that an internal error has happened, and that they should send their logs back to the developers (i.e. you). Or while developing, you might just make it print a stack trace to the console and crash the program.

For the 1st (external problem), there is no rule about what the "right" thing is to do. It all depends on the details of the application. If you want to ignore a certain condition and continue, then do so.

IN GENERAL:

It's good that you are reading technical books and articles. You can learn a lot from doing so. But please remember, as you read, you will find lots of advice from people saying that doing such-and-such a thing is always wrong or always right. Often these opinions border on religion. NEVER believe that doing things a certain way is absolutely "right" because a book or article (or an answer on SO... <cough>) told you so. There are exceptions to every rule, and the people writing those articles don't know the details of your application. You do. Make sure that what you are reading makes sense, and if it doesn't, trust yourself.

Solution 2

An empty catch block is fine in the right place - though from your sample I would say you should cetagorically NOT be using catch (Exception). You should instead catch the explicit exception that you expect to occur.

The reason for this is that, if you swallow everything, you will swallow critical defects that you weren't expecting, too. There's a world of difference between "I can't send to this email address" and "your computer is out of disk space." You don't want to keep trying to send the next 10000 emails if you're out of disk space!

The difference between "should not happen" and "don't care if it happens" is that, if it "should not happen" then, when it does happen, you don't want to swallow it silently! If it's a condition you never expected to occur, you would typically want your application to crash (or at least terminate cleanly and log profusely what's happened) so that you can identify this impossible condition.

Solution 3

If an exception should never be thrown then there is no point catching it - it should never happens and if it does you need to know about it.

If there are specific scenarios that can cause a failure that you are OK with then you should catch and test for those specific scenarios and rethrow in all other cases, for example

foreach (string address in addresses)
{
    try
    {
        addressCollection.Add(address);
    }
    catch (EmailNotSentException ex)
    {
        if (IsCausedByMissingCcAddress(ex))
        {
            // Handle this case here e.g. display a warning or just nothing
        }
        else
        {
            throw;
        }
    }
}

Note that the above code catches specific (if fictional) exceptions rather than catching Exception. I can think of very few cases where it is legitimate to catch Exception as opposed to catching some specific exception type that you are expecting to be thrown.

Solution 4

Many of the other answers give good reasons when it would be ok to catch the exception, however many classes support ways of not throwing the Exception at all.

Often these methods will have the prefix Try in front of them. Instead of throwing a exception the function returns a Boolean indicating if the task succeeded.

A good example of this is Parse vs TryParse

string s = "Potato";
int i;
if(int.TryParse(s, out i))
{
    //This code is only executed if "s" was parsed succesfully.
    aCollectionOfInts.Add(i);
}

If you try the above function in a loop and compare it with its Parse + Catch equilvilant the TryParse method will be much faster.

Solution 5

Using an empty catch block just swallows the Exception, I would always handle it, even if it is reporting back to you that an Exception occurred.

Also catching the generic Exception is bad practice due to the fact it can hide bugs in your application. For example, you may have caught an ArgumentOutOfRange exception which you did not realize was happening and then swallowed it (I.e. not done anything with it).

View more solutions

Comments

[Serberuss]

Agreed I'm quite fond on Try methods

[Serberuss]

I see what you are saying. I'm guessing your EmailNotSendException is the idea that the exceptions that are related to an invalid email address are grouped here?

[ThatBlairGuy]

+1 for that last paragraph. Errors that "should not happen" have a wonderful habit of defying expectations and occurring anyhow. Particularly after the app has been around a while and possibly gone through some revisions.

[Justin]

@Serberuss Yes - catch the most specific exception that you can

[Serberuss]

Great post thanks for the information. I agree with your last point and I'm more likely to take on information from a published source rather than a blog or internet article.

[Nick Freeman]

Yes, bad user input is not an exceptional case, it is an expected case. Humans are not as predictable as computers. Throwing an exception in this case seems to me to be a Vexing Exception more than anything else.

[Nick Freeman]

lol at making an absolute statement about absolute statements.

[Robert Kerr]

Concur on "should not happen". if they should not happen, you might realize you care after all.

[Guy Park]

I disagree on your general point. Yes, you are correct that it makes it easier to debug, but ignoring catches is just sloppy programming and shows a programmers skills. I know there are those who are those fanatics who debate if a space should go here or there, but catching all exceptions is just good programming. There are always very rare exceptions, but your 1st exception examples, should also be caught, even if to log. I would want to know if a function I called couldn't write to the disk or send an email, it would help me determine my next course of action to ensure data integrity.