wrapped exception from a component

Question

There is this style of exception system where a component throws component-specific exception. For example, all data access classes throw DataAccessException.

In this style, I often find myself having to catch and rethrow the component specific exception, because called methods are delcared as throws Exception:

try {
    int foo = foo();
    if (foo != expectedValue) {
        throw new ComponentException("bad result from foo(): " + foo);
    }
    bar();
}
catch (ComponentException e) { 
    throw e; 
}
catch (Exception e) { 
    throw new ComponentException(e); 
}

Do you find yourself doing the same? Do you find it ugly?

This question is not about validity of this style, but something within the constraints of this style.

Answer 1

That is ugly as hell. There isn't much more to comment on that ugly style, I think. If you already have all the code to handle different issues using the cause, that'd work. It's just that using

try {
  componentCall();
} catch (ComponentException e) {
  Throwable t = e.getCause();
  //Handle each possible cause
}

is less maintainable and more confusing than the other way, and the main point is that I see no advantage to using it.

Given that you are restricted to using it, I'd at least try to avoid throwing the ComponentException both with and without a cause, that makes it more confusing than it needs to be, in your example I'd add a InvalidFooException and add it as a cause for the generic ComponentException.

try {
    int foo = foo();
    if (foo != expectedValue) {
        throw new InvalidFooException("bad result from foo(): " + foo);
    }
    bar();
}
catch (Exception e) { 
    throw new ComponentException(e); 
}

getCause() is intended for chains of exceptions that are only causally instead of semantically related.

If you need them to differentiate between different exceptions (for instance, a SQLExecutionException should be corrected by a different action than an AccessDeniedException) then your method is not applicable (because forcing the use of getCause() in each catch to see what needs to be done is awful and offers no benefit than just catching the right exception out front).

If all the calling classes have to do is report an error and cancel, then the wrapping might be okay, but I wouldn't do it, as it adds little benefit and if you later need to differentiate will make you rewrite lots of stuff.

What is useful is to create a hierarchy of exceptions for exception of the same type, in the lines of:

If DataAccessException would be the root, then you could for example have DataSourceUnavailableException, InvalidDataSourceException, InvalidDataException and then you can decide to catch either parent only (if the action is the same) or to catch each exception separately.

Why do you need this to behave this way, by the way?

Answer 2

It would maybe less ugly to do so (if the function foo() is the one declared to throw Exception) :

int foo;
try {
    foo = foo();
}
catch (Exception e) { 
    throw new ComponentException(e); 
}
if (foo != expectedValue) {
    throw new ComponentException("bad result from foo(): " + foo);
}
bar();