How do I refactor closing a stream in Java?

Question

Due to my company's policy of using Eclipse and using Eclipse's code-autofix, the following code pattern appears excessively in the codebase:

InputStream is = null;
try {
    is = url.openConnection().getInputStream();
    // .....
} catch (IOException e) {
    // handle error
} finally {
    if (is != null) {
        try {
            is.close();
        } catch (IOException e) {
            // handle error
       }
    }
}

IMO it's extremely fugly and hard to read, especially the portion within the finally block (is there really a need to catch 2 instances of IOException?). Is there anyway to streamline the code such that it looks cleaner?

Answer 1

See this question, use the closeQuietly() solution.

InputStream is = null;
try {
    is = url.openConnection().getInputStream();
    // .....
} catch (IOException e) {
    // handle error
} finally {
    IoUtils.closeQuietly(is);
}

// stolen from the cited question above
public class IoUtils {

  public static closeQuietly (Closeable closeable) {
    try {
      closeable.close();
    } catch (IOException logAndContinue) {
      ...
    }
  }
}

Or wait for JDK7's ARM blocks.

Answer 2

First, about using IOUtils - may worth a shot telling your supervisors that the very application-server / Java runtime environment they might use, uses IOUtils and similar libraries itself. so in essence you're not introducing new components to your architecture.

Second, no, not really. There isn't really any way around it other than writing your own utility that will immitate IOUtils' closeQuietly method.

Answer 3

public class Util {
    public static void closeStream(inputStream is) {
        if (is != null) {
            try {
               is.close();
            } catch (IOException e) {
               // log something
        }
    }
}

Now your code is

InputStream is = null;
try {
    is = url.openConnection().getInputStream();
    // .....
} catch (IOException e) {
    // handle error
} finally {
    Util.closeStream(is);
}

Not a lot else to do as the IOException in the catch might have some specific processing.

Answer 4

Why do anything? It's working code. It's correct.

Leave it be.

Answer 5

You could define something like this somewhere:

private static interface InputStreamCallback {

    public void doIt(InputStream is) throws IOException;

}

private void with(InputStreamCallback cb) {

    InputStream is = null;

    // Creational code. Possibly adding an argument

    try {
        cb.doIt(is);
    } catch (IOException e) {
        // handle error or rethrow.
        // If rethrow add throws to method spec.
    } finally {
        if (is != null) {
            try {
                is.close();
            } catch (IOException e) {
                // handle error or rethrow.
            }
        }
    }
}

And invoke your code like this:

with(new InputStreamCallback() {

    @Override
    public void doIt(InputStream is) throws IOException {
        is = url.openConnection().getInputStream();
        // .....
    }

});

If you declare with method static in a helper class, then you could even do an import static of it.

There's a drawback. You need to declare url final.

EDIT: creational code is not the point. You can arrange it in several ways. The callback is the point. You could isolate what you need to do there.