Question

I would like opinions about how should I handle my exceptions and still avoiding duplicate code. Here is how I thought to handle the situation.

The duplicate code avoiding situation ( I don't like using instanceof here).

try{

    //some code which might throw multiple different exceptions. 

} catch(Exception e) {

    //do something here

    if(e instanceof IOException) {
       // do something only when this type exception occurred.
    } else if( e instanceof SQLException){
      // do something only when this type of exception occurred.
    } else if( e instanceof SomeCustomExceptionMaybe){
       // do something only when this type of exception occurred.
    }

   //continue exception handling here.
} 

vs.

No instanceof situation ( I realy don't like having duplicate code).

try{

     //some code which might throw multiple different exceptions.

} catch(IOException e1) {
    // The order must always be this.
    // do something general for each exception
    // do something only with this exception 
    // do something general again.
} catch(SomeCustomExceptionMaybe e2) {
    // The order must always be this.
    // do something general for each exception
    // do something only with this exception 
    // do something general again.
} //and so on

ps : Try to avoid the java 7 exception handling when giving an answer :

catch(IOException | SomeOtherTypeException | AnotherTypeException)

edit: I'm not using java 7 that's why I asked to avoid responses based on java 7 .

Was it helpful?

Solution

The first, obvious solution is to use methods:

catch(IOException e1) {
    doSomethingGeneral(); 
    // do something only with this exception 
    doSomethingGeneralAgain();
}

That leaves some duplication though. At the price of verbosity, since Java doesn't have lambdas yet (but will in Java 8), you can extract the specific behavior inside some anonymous class:

catch(IOException e1) {
    handleException(new Runnable() {
        @Override
        public void run() {
            // do something only with this exception
        }
    });
}

where handleException() would work like this:

private void handleException(Runnable specificBehavior) {
    // do something general
    specificBehavior.run();
    // do something general again
}

I can't wait for Java 8, which will allow rewriting this as:

catch(IOException e1) {
    handleException(() -> {
        // do something specific
    });
}

OTHER TIPS

Catching a raw Exception, like in your first example, is a very bad solution. Exception is the parent class for checked exceptions (IOException, etc.) that should be caught and dealt with ; but also for RuntimeExceptions, that signal programming errors (didn't check for reference nullity or array index, etc.) that should generally not be captured - rather corrected.

Furthermore, catching a very broad Exception may have some unintended side-effects when the code in the try block is later modified and throws new kinds of exceptions : they would be silently caught by the existing catch block.

Therefore, I think it is much better to have a distinct catch block for each different type of exception, even if it means duplicating (pre-Java7) some error handling code.

Of course, the most obvious way to limit this code duplication is to externalize it in a common method :

try {
    ... 
} catch (FooException fe) {
    handleException(fe);
} catch (BarException be) {
    handleException(be);
} 
...

private void handleException(Exception e) {
    ...
}

Which is OK if all the exceptions are handled strictly in the same way. But as soon as you want to do different things based on the exact exception type, you'll need to use the instanceof operator, which is always a strong code smell.

In conclusion, I would personnaly keep the error handling code in the catch blocks, even if it means some code duplication for now - it'll still be more robust in the long term.

I've not tested the following approach yet, but maybe it helps:

try{

     //some code which might throw multiple different exceptions.

} catch(Exception e) {
    if ((e instanceof IOException) || (e instanceof SomeCustomExceptionMaybe)) {
        //do something
    } else {

    }
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top