Android Java - how to refactor out a try-catch-finally with a throw in the finally, all inside an abstract class?

Question

Good afternoon,

I'm currently working with the code from:

https://github.com/kevinsawicki/http-request

I'm attempting to refactor the code as applicable to clear all the Android Studio warnings before I include it in a project I'm working on. Currently I'm working with the following nested abstract class:

///////////////////////////////////////////////////////////////////////////////////////////////
protected static abstract class Operation<V> implements Callable<V> {

    protected abstract V run() throws HttpRequestException, IOException;

    protected abstract void done() throws IOException;

    public V call() throws HttpRequestException {
        Log.d(TAG, "in HttpRequest nested class Operation call");
        boolean thrown = false;

        try {
            return run();
        } catch (HttpRequestException e) {
            thrown = true;
            throw e;
        } catch (IOException e) {
            thrown = true;
            throw new HttpRequestException(e);
        } finally {
            try {
                done();
            } catch (IOException e) {
                if (!thrown) {
                    throw new HttpRequestException(e);
                }
            }
        }
    }
}   // end Operation

This is producing the following warning for having a throw inside a finally block:

I've been looking at this for a while but I can't seem to find a way to factor out this warning. I did see this other answer:

throws Exception in finally blocks

However I would really prefer to not introduce another function. If I was to introduce a closeQuietly function, would that go inside or outside the nested class listed above? Please advise, thanks!

Chris Thompson · Accepted Answer · 2016-08-14T15:42:13.073

It's just a warning. If you read the explanation it says (emphasis mine)

While occasionally intended, such throw statements may mask exceptions thrown and tremendously complicate debugging.

If you need to do it, then do it, but just make sure it's actually what you want to do and understand the implications (it's akin to saying "do you really want to do this?!"). Not all of IntelliJ's warnings can be eliminated.

Edit based on follow up: You have to ask yourself if your framework needs to throw that exception in the finally block. You can implement a similar approach to what was linked without using another function (just replace the throw statement in finally with a log statement), but that might not be desirable. It depends entirely on the potential error conditions.

If, for example, you're expecting done() to run into issues whenever you've previously received an IOException or an HttpRequestException then you probably don't need to throw anything in the finally block (just log it). But, if you need to make sure you alert the user if something went wrong trying to clean up, then you do need to throw there and you should ignore the warning.

I'm aware its a warning, my goal before putting this into productions is to remove as many warnings as possible. Can you suggest a refactor to achieve this? — cdahms, Aug 14 '16 at 15:36

score 1 · Answer 2 · answered Aug 14 '16 at 15:36

I suppose you could do something like this:

protected static abstract class Operation<V> implements Callable<V> {

    protected abstract V run() throws HttpRequestException, IOException;

    protected abstract void done() throws IOException;

    public V call() throws HttpRequestException {
        Log.d(TAG, "in HttpRequest nested class Operation call");
        boolean thrown = false;

        try {
            return run();
        } catch (IOException e) {
            throw new HttpRequestException(e);
        } finally {
            try {
                done();
            } catch (IOException e) {
                // handle the IOException
            }
        }
    }
}   // end Operation

If an HttpRequestException is ever thrown it'll be thrown by the method, you still transform the IOException into an HttpRequestException (not quite sure why you want to do that), and in the finally block you would need to catch and handle the IOException appropriately.

Android Java - how to refactor out a try-catch-finally with a throw in the finally, all inside an abstract class?

2 Answers2