Throwing the Exception But Using the Finally Block

Question

Does this piece of code make sense? The idea is to throw any exceptions that may occur but always run the finally block to close the streams.

private void streamToFile(HttpResponse transferResponse) throws Exception {
    OutputStream output = null;
    InputStream input = null;
    try {
        input = new BufferedInputStream(transferResponse.getEntity().getContent());
        output = new FileOutputStream(transferFile);
        final int BUFFER_SIZE = 1024*10;
        byte data[] = new byte[BUFFER_SIZE];
        int count;
        while ((count = input.read(data)) != -1) {
            output.write(data, 0, count);
            publishProgress(count);
            remainingBytes = remainingBytes - count;
        }
    } finally {
        input.close();
        output.close();
    }
}

Answer 1

You should use the closeQuietly methods of IOUtils (Apache Commons IO) (or similar ones). It ignores exceptions and accepts null values.

Its implementation is just a few lines of code, so you can copy it directly to your Android application without the jar file:

public static void closeQuietly(Closeable closeable) {
    try {
        if (closeable != null) {
            closeable.close();
        }
    } catch (IOException ioe) {
        // ignore
    }
}

It accepts null values which means that you can call it with null, the closeQuietly will handle that and you don't have to check null values in the finally block:

} finally {
    if (input != null) {
        closeQuietly(input);
    }
    if (output != null) {
        closeQuietly(output);
    }
}

Instead of this the following is completely enough:

} finally {
    closeQuietly(input);
    closeQuietly(output);
}

It's much simpler.

closeQuietly ignores exceptions which means that it catches them but don't do anything with them. Notice the empty catch block:

    } catch (IOException ioe) {
        // ignore
    }

It ensures that if closeable.close() throws an IOException the closeQuietly(output) will run and it will close the output stream too.

I'm a little bit paranoid about this so I always put a logging statement to the catch block:

    } catch (IOException ioe) {
        logger.warn("Could not close the stream", ioe);
    }

To be honest, I've never seen this exception, so I think it's safe to left this block empty.

Answer 2

Yes this is the right way to do it but you should check output and input variables for null value before calling on them the close() method.