Are try/catch for every single statement that throws an exception considered an anti-pattern?

Question

I am currently reviewing a colleagues Java code, and I see a lot of cases where every single statement that may throw an exception being encapsulated in its own try/catch. Where the catch block all perform the same operation (which operation is not relevant for my question).

To me this seems like a code smell, and I do remember reading about it being a common anti-pattern. However I cannot find any references on this.

So are try/catch for every single statement that throws and exception considered an anti-pattern, and what are the argument supporting this?

---

Constructed example: (Does not relate to the original problem, so please don't mind other problems with this example, as it is just constructed to illustrate what I mean.)

public int foo()
{
    int x, y = 7;

    try
    {
        x = bar(y);
    }
    catch(SomeException e)
    {
        return 0;
    }

    try
    {
        y = baz(x,y);
    }
    catch(SomeOtherException e)
    {
        return 0;
    }

    /* etc. */

    return y;
}

(Assume that it is appropriate to catch both exceptions here, i.e. we know what do with them, and the appropriate thing is to return 0 in both cases.)

Answer 1

I can't serve you with an authoritative source, but these are fundamental tenets of exception handling:

• exceptions should be caught at the point where one can properly handle them, and
• you must not swallow exceptions - there should always be (at least) a trace of an exception thrown, so the minimum is to log it, so that at least the developers get a chance of noticing that something bad happened. Which leads to the third point:
• exceptions should only be used to signal exceptional bad events, not to control program flow.

There are lots of earlier posts on SO dealing with this, for example Best practices for exception management in JAVA or C#.

Answer 2

My initial reaction was that this is a bad thing. (Also I initially had arguments about catching exceptions in general, but removed those to focus on the example given in the question.) Returning 0 on an exception is hard to judge here because it's unknown what bar and baz do, and what the exceptions represent. If the caller really doesn't need to know anything went wrong, and the caller doesn't have to distinguish between the return value as error condition and return value as data, then this is ok. On the other hand if the exceptions thrown are problems with resources not being available it is better to fail fast than try to struggle on.

I am doubtful this is a good thing because if it is reasonable for this code to return 0 on an exception, it would have been at least as reasonable for bar and baz to return 0 in the first place.

To address catching exception for every single statement in general (as opposed to returning immediately with a default value as the example does), that is a bad thing because once you have an exception thrown, something has gone wrong and your method or object can be in a bad state. The purpose of exception-handling is to have a means of escaping a context where something has gone wrong and getting back to a place with a known state. If catching every single exception and blundering on was ok, all our languages would support `On Error Resume Next'.

Answer 3

The devil is in the detail. It depends on the context. When do you expect these exceptions? For example if SomeException is indicative of a business rule violation that is quite common, and returning 0 is valid in that context, the I can see the above code being quite valid. Having the try..catch block close to the method that causes it is not a bad thing, as it could be quite a chore to work out which method could cause an exception if you wrap 10 methods in one giant try..catch.

However, if these are exceptions that are indicative of something having gone very wrong and SomeException shouldn't occur, then returning "0" as a magic error indicator hides could potentially hide that something has gone wrong, the it could be a maintenance nightmare to find out where the problem occurred.

If the exception is not something that you can recover from, then you might as well put it in the throws clause or wrap it and rethrow it (if the exception is implementation specific, I wouldn't just put it in the throws clause as you wouldn't want to pollute a clean interface with Exceptions that are specific to the implementation). See also this SO article on checked vs unchecked exceptions which is applicable.

Not logging an exception is also not necessarily a bad thing. If the exception occurs very frequently and is not an indicator of something having gone terribly wrong, then you wouldn't want to write to the log every time as otherwise you'll "poison the logs" (i.e. because if 99% of log entries are related to SomeException, then what's the likelihood that you'd miss an exception in the log and/or you run out of disk space because you get the log message 50,000 times a day).

Answer 4

For the given example, the argument is that it is redundant. You need just one try/catch block with just one return null.

Answer 5

I actually don't really think its an anti-pattern, per-se. But its definitely not clean code, which is almost as bad.

The reason that I don't believe its an anti-pattern is that it doesn't seem like a good idea to begin with, which is a requirement for anything to be an anti-pattern. Furthermore, this is a solution to coding problem whereas anti-patterns are general (bad) solutions to architectural problems.

Answer 6

I think your colleague has read the old J2EE best practise "Catch an exception as close as possible to its source." and then he went bit overboard with it.