24

I'm refactoring some code to use guava Cache.

Initial code:

public Post getPost(Integer key) throws SQLException, IOException {
    return PostsDB.findPostByID(key);
}

In order not to break something I need to preserve any thrown exception as is, without wrapping it.

Current solution appears somewhat ugly:

public Post getPost(final Integer key) throws SQLException, IOException {
    try {
        return cache.get(key, new Callable<Post>() {
            @Override
            public Post call() throws Exception {
                return PostsDB.findPostByID(key);
            }
        });
    } catch (ExecutionException e) {
        Throwable cause = e.getCause();
        if (cause instanceof SQLException) {
            throw (SQLException) cause;
        } else if (cause instanceof IOException) {
            throw (IOException) cause;
        } else if (cause instanceof RuntimeException) {
            throw (RuntimeException) cause;
        } else if (cause instanceof Error) {
            throw (Error) cause;
        } else {
            throw new IllegalStateException(e);
        }
    }
}

Is there any possible way to make it nicer?

Vadzim
  • 24,954
  • 11
  • 143
  • 151

2 Answers2

39

Just after writing the question started thinking about utility method powered with generics. Then remembered something about Throwables. And yes, it's already there! )

It may also be necessary to handle UncheckedExecutionException or even ExecutionError.

So the solution is:

public Post getPost(final Integer key) throws SQLException, IOException {
    try {
        return cache.get(key, new Callable<Post>() {
            @Override
            public Post call() throws Exception {
                return PostsDB.findPostByID(key);
            }
        });
    } catch (ExecutionException e) {
        Throwables.propagateIfPossible(
            e.getCause(), SQLException.class, IOException.class);
        throw new IllegalStateException(e);
    } catch (UncheckedExecutionException e) {
        Throwables.throwIfUnchecked(e.getCause());
        throw new IllegalStateException(e);
    }
}

Very nice!

See also ThrowablesExplained, LoadingCache.getUnchecked and Why we deprecated Throwables.propagate.

Vadzim
  • 24,954
  • 11
  • 143
  • 151
  • Hesitated if self-answered question should be posted at all. But this made it clear: http://meta.stackexchange.com/questions/2706/posting-and-answering-questions-you-have-already-found-the-answer-to – Vadzim Dec 27 '11 at 15:02
  • 1
    Note that if e.getCause() is a RuntimeException or an Error, the above code will throw that instead of an IllegalStateException(e). The code in this answer matches the original code in the question but, if for some strange reason, one always want to wrap ALL non-SQL and non-IO exceptions into an IllegalStateException, you will need the custom code. – Niraj Tolia Jan 04 '12 at 06:15
  • @Niraj, sure. `throw new IllegalStateException(e);` here is dead code line just to make the compiler happy. – Vadzim May 04 '16 at 16:16
-1

Just use @SneakyThrows from Lombok. No problem anymore with forced exception wrapping.

<rant> It's 2021 and Java still has checked exceptions... When will people get it that, even if checked exceptions look good on paper, they create way too many problems in practice?

Long term solution: move to a proper language, like Kotlin, if you have the opportunity. </rant>