Multiple specific catch or one catches all?

Question

I've seen this topic arise sometimes in the past but even after Googling about it, I still can't figure out what would be a good and elegant way to deal with it, so here it goes.

Say I have some code that throws various exceptions...

try {
  /* some code that throws these exceptions */
} catch (NoSuchAuthorityCodeException e) {
    throw new MyAPIException("Something went wrong", e);
} catch (FactoryException e) {
    throw new MyAPIException("Something went wrong", e);
} catch (MismatchedDimensionException e) {
    throw new MyAPIException("Something went wrong", e);
} catch (TransformException e) {
    throw new MyAPIException("Something went wrong", e);
}

... and as we can see, I just wrap these exceptions and throw a new one saying that something went wrong within my API.

This seems to me a overly repetitive code, so one would simply catch an Exception type and handle wrap it and throw a new one.

try {
  /* some code that throws these exceptions */
} catch (Exception e) {
    throw new MyAPIException("Something went wrong", e);
}

In this case it stays way more simpler, but the thing is we'd be catch also every RuntimeException. Given this, we could catch-re-throw the RuntimeException so we could avoid this.

try {
  /* some code that throws some exceptions */
} catch (RuntimeException e) {
    throw e;
} catch (Exception e) {
    throw new MyAPIException("Something went wrong", e);
}

Its a bit bulky but it does the trick. Now theres another little issue about the catch(Exception e), that if my inner API throws another MyAPIException it would also be caught, wrapped and throwed within another MyAPIException. In this particular case we could also catch MyAPIException and re-throw it.

try {
  /* some code that throws some exceptions */
} catch (RuntimeException e) {
    throw e;
} catch (MyAPIException e) {
    throw e;
} catch (Exception e) {
    throw new MyAPIException("Something went wrong", e);
}

Well, its getting messy again, but in this case we prevent wrapping MyAPIException and simply re-throw it. But, theres also another issue with the catch (Exception e) block that is if the inner API changes and start throwing another kind of exception (some other than these 4 mentioned above), the compiler wouldn't say anything about it and we wouldn't have a clue. Not that this would be a major problem, since probably I would treat it in the same manner.

With this scenario, I think the question is, which one is better and are there better options?

Answer 1

Below is another approach and a usage example. The MyAPIException class has a handle method. The handle method will handle any Exception.

1. If the Exception is a Runnable or a MyAPIException, handle will throw it without wrapping it.
2. If not, the handle method will test whether assertions are enabled. If assertions are enabled, the handle method will test to see if the exception is assignable from one of the supported exception types, throwing an AssertionError if it is not assignable. (If assertions are not enabled, the handle method ignores the supported exception types parameter.)
3. Finally - if it reaches this point - the handle method will wrap the exception in a MyAPIException and throw it.

When you test your code, run it with assertions enabled. In production, run it with assertions disabled.

<soapbox> If you use this technique, you will have to test for errors that the compiler would have otherwise caught.</soapbox>

class MyAPIException extends Exception
{
    private static final long serialVersionUID = 0 ;

    MyAPIException ( Throwable cause )
    {
        super ( cause ) ;
    }

    static void handle ( Exception cause , Class < ? > ... supportedExceptionTypes ) throws MyAPIException
    {
        try
        {
        throw ( cause ) ;
        }
        catch ( RuntimeException e )
        {
        throw ( e ) ;
        }
        catch ( MyAPIException e )
        {
        throw ( e ) ;
        }
        catch ( Exception e )
        {
        search :
        try
            {
            assert false ;
            }
        catch ( AssertionError a )
            {
            for ( Class < ? > c : supportedExceptionTypes )
                {
                if ( c . isAssignableFrom ( e . getClass ( ) ) )
                    {
                    break search ;
                    }
                }
            assert false : e + " is not one of the supportedExceptionTypes : " + supportedExceptionTypes ;
            }
        MyAPIException error = new MyAPIException ( e ) ;
        throw ( error ) ;
        }
    }
}

This is an example of usage. You can run it with assertions enabled/disabled and with parameters 1,2,3,4 to see how it handles different situations.

class Usage
{
    public static void main ( String [ ] args ) throws MyAPIException
    {
    try
        {
        switch ( Integer . parseInt ( args [ 0 ] ) )
            {
            case 1 :
            throw new RuntimeException ( ) ;
            case 2 :
            throw new SQLException ( ) ;
            case 3 :
            throw new MyAPIException ( null ) ;
            case 4 :
            throw new IOException ( ) ;
            }
        }
    catch ( Exception cause )
        {
        System . out . println ( cause . getMessage ( ) ) ;
        System . out . println ( cause . getCause ( ) ) ;
        MyAPIException . handle ( cause , IOException . class ) ;
        }
    }
}

Answer 2

OK guys, here is what I came up with... It's not that elegant but I think is a bit better than having various catches.

First, we have an abstract class called ExceptionHandler.

package my.api.pack;

import java.lang.reflect.ParameterizedType;
import java.util.LinkedHashSet;
import java.util.Set;

public abstract class ExceptionHandler<E extends Exception> {

    Set<Class<? extends Exception>> exceptionClasses = new LinkedHashSet<Class<? extends Exception>>();

    protected abstract void handler(Throwable t) throws E;

    public ExceptionHandler<E> catches(Class<? extends Exception> clazz) {
        exceptionClasses.add(clazz);
        return this;
    }

    @SuppressWarnings("unchecked")
    private Class<E> getGenericsClass() {
        ParameterizedType parameterizedType = (ParameterizedType) getClass().getGenericSuperclass();
        return (Class<E>) parameterizedType.getActualTypeArguments()[0];
    }

    @SuppressWarnings("unchecked")
    public final void handle(Throwable t) throws E, UnhandledException {
        if (getGenericsClass().isInstance(t)) {
            throw (E) t;
        }

        for (Class<? extends Exception> clazz : exceptionClasses) {
            if (clazz.isInstance(t)) {
                handler(t);
                return;
            }
        }

        throw new UnhandledException("Unhandled exception", t);
    }

}

Along with it, we have this simple runtime exception called UnhandledException

package my.api.pack;

public class UnhandledException extends RuntimeException {

    private static final long serialVersionUID = -3187734714363068446L;

    public UnhandledException(String message, Throwable cause) {
        super(message, cause);
    }


}

With it we can use these to handle exceptions like this...

try {
  /* some code that throws these exceptions */
} catch (Exception e) {
    new ExceptionHandler<MyAPIException>() {
        @Override
        protected void handler(Throwable t) throws MyAPIException {
            throw new MyAPIException("Something went wrong", t);
        }
    }.
        catches(MismatchedDimensionException.class).
        catches(NoSuchAuthorityCodeException.class).
        catches(FactoryException.class).
        catches(TransformException.class).
        handle(e);
    return null;
}

What you guys think?

Answer 3

It all depends what you you want to do. If all you want to do is throw new MyException("Something went wrong", e); then one catch all will do.

Answer 4

Since you are stuck with Java5 and you use a proprietary exception, why not put all that exception logic into the exception class.

Useage

try
{
     // some code that might throw one of several exceptions
}
catch ( Exception cause )
{
     MyAPIException . handle ( cause ) ;
}

MyAPIException contains the logic

class MyAPIException extends Exception
{
    private MyAPIException ( String message , Throwable cause ) { super ( message , cause ) ; }

    private static void myAPIException ( Exception cause ) throws MyAPIException
    {
         throw new MyAPIException ( "Something Went Wrong" , cause ) ;
    }

    public static void handle ( Exception e ) throws MyAPIException
    {
          try
          {
               throw ( e ) ;
          }
          catch ( RuntimeException cause )
          {
                throw cause ;
          }
          catch ( MyAPIException cause )
          {
                 throw cause ;
          }
          catch ( NoSuchAuthorityCodeException cause )  // repeat for other exceptions
          {
                 myAPIException ( cause ) ;
          }
          catch ( Exception cause ) // this should not happen
          {
                  assert false ; // or log it or throw a RuntimeException ... somehow signal that programming logic has failed
          }
    }
}

Answer 5

In JAVA 7 you will be able to do something like

try {
  /* some code that throws these exceptions */
} catch (NoSuchAuthorityCodeException , FactoryException, MismatchedDimensionException , TransformException e) {
    throw new MyAPIException("Something went wrong", e);
}

So perhaps the best answer is to chill out for a while and then upgrade to JAVA 7 when it becomes available.

Answer 6

Have more than one MyApiException. If you have every exception as MyApiException, it becomes a bit difficult for you and for others too to read your code. Naming it properly would also be crucial. Plus you do not always want to catch and rethrow it. If thats the case, simply declare a throws in method signature.

Plus you cannot fix on either of a simple catch or multiple catches. Its more of a judgmental decision IMO as there are exceptions which are fatal (as in the entire program has to halt) to more easy-to-handle types.

I do not see anything wrong at all when you have a bunch of throws clauses in your API. It is a much neater implementation. So what if the caller has to handle the exceptions that you've defined. If a method in your API may throw an exception and something has to be done about it, then the caller should have to handle it. Anyways, your documentation of that particular exception has to be clear in order not to make the caller confused.

One more factor that decides this is the complexity of the API. Sometime back I had written an API which was moderately complex and I had to deal with the same issues like the one that you've presented. I had written few custom exceptions for few methods. I am sure you will not end up throwing exceptions for all the public method that the API exposes, but there are places which they are inevitable.

At last, if you feel having too many custom exceptions a bitter choice, you can have one single exception with clearly documented error codes. This way the caller can handle the exception and deal with the error codes as he sees fit.