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?

Answer 1

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.

Answer 2

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.

Answer 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.

Answer 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.