3

Often, when implementing a template method or interface method, you can only throw one specific type of exception defined by the method. But your implementation may make class to API's that throw an incompatible exception type, or many different exception types.

Naturally you need to catch them and wrap the exceptions into the type suitable for the implemented method signature. Lets assume we want to implement this interface:

public interface SomeDataGetter {

    public long getSomeData() throws IOException;

}

Our implementation makes use of some other API product to implement this, and the API method we are calling may have this signature:

public long loadFromDBOrCache(Object ... params) throws SQLException, IOException, ObjectNotFoundException, RuntimeException, FridayException, NotWeekendException, NumberIs42Exception;

I made this up to demonstrate the case where you can't exactly enumerate all the possibly thrown exceptions by concrete type. Do note that IOException is a type we are allowed to throw from our implementation.

Now I can go the lazy route when implementing this and wrap anything to fit my signature:

@Override
public long getSomeData() throws IOException {
    try {
        return loadFromDB(...);
    } catch (Exception e) {
        throw new IOException(e.getMessage(), e);
    }
}

This will obviously wrap any exception into an IOException (even an IOException) and it works out ok. But I'd like to not wrap IOExceptions, since I am allowed to throw those without wrapping them:

@Override
public long getSomeData() throws IOException {
    try {
        return loadFromDB(...);
    } catch (IOException e) {
        throw e;
    } catch (Exception e) {
        throw new IOException(e.getMessage(), e);
    }
}

You can imagine this gets cumbersome quickly if there are multiple possible exception in the implementation and multiple exceptions you are allowed from the implementation. I need an extra catch for each exception I want to pass throgh.

Whats the best idiom to keep that readable (also, I'm lazy, and don't want to write all these extra catches) and still avoid unneccessary exception nesting? Or shoud I not bother and just wrap everything?

Durandal
  • 19,919
  • 4
  • 36
  • 70

2 Answers2

2

One approach would be making a method that wraps all "prohibited" exceptions in an allowed one, while returning all the allowed ones unwrapped, like this:

private static void throwIoException(Exception e)
    throws IOException // <<= Add other "allowed" exceptions here
{
    if (e instanceof IOException) {
        throw (IOException)e;
    }
    ... // <<= Add checks for other "allowed" exceptions here
    throw new IOException(e.getMessage(), e);
}

Now you can use a single catch block, and do the wrapping as needed:

try {
    return loadFromDB(...);
} catch (Exception e) {
    throwIoException(e);
}

One unpleasant consequence of this is that the stack trace shows the utility method at the top of the newly created IOException, but that's not important, because the real exception is the wrapped one, not the IOException wrapper. If the exception that you caught happens to be IOException, the correct stack trace should remain in place.

Community
  • 1
  • 1
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Hm, not exactly perfect for my lazyness - but handy when there are many instances of such catch cascades. The utility method appearing in the stack trace doesn't bother me much (the JRE uses that a lot, too). But it could even be avoided entirely by changing "throwIOException(ex)" to "throw wrapAsIOExcption(ex)". – Durandal Oct 01 '13 at 16:21
0

I would consider the lazy route of wrapping all exceptions you get into IOExceptions (or another checked exception) to be a bad practice. Instead I would consider wrapping the exceptions in runtime exceptions, thereby bypassing the catch or specify requirement. E.g.

@Override
public long getSomeData() throws IOException {
    try {
        return loadFromDB(...);
    } catch (Exception e) {
        throw new RuntimeException(e.getMessage(), e);
    }
}

The reason why this is better is that checked exceptions carry a certian meaning. If you catch for instance a ParseException in your code and rethrow that as a IOException you are lying. As a user of your code I might be able to do something about certain types of checked exceptions, but if you obfuscate the true cause of an exception it will be more difficult to debug the code when an error occurs.

In general I think you should minimize the use of checked exceptions since it litters error handling code throughout your application. Also if you are using someone else's code you have no guarantee that a RuntimeException won't be thrown anyway (unless you carefully read through it all). Therefore you have to consider that possibility anyway and handle it somewhere so your application don't crash. The virtues of unchecked exception vs checked exceptions has been discussed quite a lot elsewhere here and here for instance.

Community
  • 1
  • 1
Emil L
  • 20,219
  • 3
  • 44
  • 65
  • 3
    Wouldn't that be *especially* bad, considering the caller *may* code to respond to an IOException while letting the RuntimeException bubble up (since the method signature explicitly declares the IOException). If *I* were the caller I would probably catch the IOException, take the message and display it to the user (with some additional text). On the other hand I would let the RuntimeException pass up to crash the application or whatever. – Durandal Sep 27 '13 at 18:27
  • @Durandal Sure, it might be nicer to subclass `RuntimeException` into `RuntimeIOException` and document that the caller would need to handle that. But converting form one type of checked exception to another is bad, see my edit for why. – Emil L Sep 27 '13 at 18:52