Catch separate exceptions or use instanceof - Java 6

Question

Assume that this piece of code is in 20 places and always the same

try {
    // do something
} catch (FirstException e) {
    // log it
} catch (SecondException e) {
    // log it
}

Wouldn't be better to use something like this or instanceof is not good solution?

try {
    // do something
} catch(Exception e) {
    logException(e);
}

void logException(Exception e) {
    if (e instanceof FirstException) {
        // log it
    } else if (e instanceof SecondException) {
        // log it differently
    } else {
        // do something with other exception 
    }
}

The only thing that I really hate about the solution is catching Exception which is definitelly not the best way... Is there any better way?

I'd use the first approach and call the logException for all the exceptions caught. — Thihara
– Thihara, Commented Mar 21, 2013 at 8:48
Catching each exception separately makes sense only when you intend to do different processing for each. In real world applications, this is rarely the case. The feasible approach is to catch em' all using the superclass Exception or Throwable if runtime errors are a major concern and log them correctly so as to make sense to the developer where and what actually happened. — Vrushank
– Vrushank, Commented Mar 21, 2013 at 9:08

Marko Topolnik · Accepted Answer · 2013-03-21 09:45:16Z

10

In Java 7, use catch (FirstException1 | SecondException | ...)
There may be nothing wrong with catch (Exception e)—you do want to log all exceptions, don't you? I would in fact advise catch (Throwable t) because OutOfMemoryError s and StackOverflowErrors also want to be logged.

An advice from many years of experience with logging exceptions is to log them all the same way. The exception message is enough as human-readable text, and what the developer really needs for debugging is the stack trace.

Just be careful about one thing: never catch exceptions too early: catch them at a single place for the whole application, the so-called exception barrier—it is at the level where you enter and exit a unit of work.

If checked exceptions are giving you trouble at the lower level, wrap them into RuntimeException:

try {
  ...
} 
catch (RuntimeException e) {throw e;} 
catch (Exception e) {throw new RuntimeException(e);}

Only if you know precisely and in advance that there is an exception which has business-level meaning to your application, and will not abort the current unit of work, but redirect its flow, is it appropriate to catch that exception at a lower level. In practice such exceptions are rare compared to the totality of all possible exceptions thrown by the application code.

edited Mar 21, 2013 at 9:45

answered Mar 21, 2013 at 8:50

Marko Topolnik

201k31 gold badges336 silver badges455 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

user219882 Over a year ago

Isn't catching Throwable like the greatest evil of all? Because I do not want to catch all the exceptions... just the two

Marko Topolnik Over a year ago

Only if you catch them too early---as I explain in the edited answer.

Jasper Blues Over a year ago

Thanks - didn't know you could catch exception a | b in java 7 (mostly working in Obj-C lately). . . handy tip.

AlexR · Accepted Answer · 2013-03-21 08:50:19Z

1

The first approach is definitely better. Generally it is a bad practice to catch Exception because in this case you catch RuntimeExceptions too.

answered Mar 21, 2013 at 8:50

AlexR

116k16 gold badges137 silver badges216 bronze badges

Comments

VishalDevgire · Accepted Answer · 2013-03-21 08:50:20Z

1

Former is clean and great solution if you only have to log the exceptions.

Else first approach is better.

answered Mar 21, 2013 at 8:50

VishalDevgire

4,28811 gold badges36 silver badges63 bronze badges

Comments

Jasper Blues · Accepted Answer · 2013-03-21 08:54:04Z

In the book "Refactoring to Patterns" one of the common refactorings is "replace instanceof with polymorphism" - in other words whenever you use instanceof, consider if polymporphism would in fact work better. . .

Having said that, for this particular question the Spring philosophy of replacing checked exceptions with runtime exceptions springs to mind (excuse the pun).

The idea is that checked exceptions can be overused - is the exception something that could be recovered from? If yes, ok. . . if not, just let it propagate up the chain. You can do this by:

Re-throwing it. . . (but better still)
Wrap it in a RuntimeException

Create a logging Aspect:

Another thing to consider is that if you do need to log these exceptions at precisely the point they occur rather than having them propagate up the chain, and that they occur in 20 different places, then they're a cross-cutting concern. . . You could have the regular method just rethrow the exception, and then write an aspect to catch and log them. . . . again using Spring makes this easy.

Collectives™ on Stack Overflow

Catch separate exceptions or use instanceof - Java 6

4 Answers 4

3 Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related