Do you need to close streams?

Question

This question comes in two parts:

What is the difference between InputStream i1 = new InputStream() and new InputStream()?
and
Is it worth it to create all local variables just to close them?

I know the simple answers with the first you can keep the variable, continue to use the variable and even close the input stream like a good programmer. The second you have lost its reference, but it looks more concise. Is there any memory difference between the two? How about speed difference (in destruction)?

Now to the examples that spurred my thoughts. First we use `'new Object()` with no regards to throwing it away.

 public void getLongStrings() throws IOException {
        try {
            foo = FileCopyUtils.copyToString(new InputStreamReader(aBook.getInputStream()));
            bar = FileCopyUtils.copyToString(new InputStreamReader(aNovel.getInputStream()));
        }
        catch (IOException ioe) {
            //do something appropriate here;
        }
    }

Now for the more verbose method

public void getLongStrings() throws IOException {
        InputStream i1 = null;
        InputStream i2 = null;
        InputStreamReader isr1 = null;
        InputStreamReader isr2 = null;
        try {
            i1 = aBook.getInputStream();
            i2 = aNovel.getInputStream();
            isr1 = new InputStreamReader(i1);
            isr2 = new InputStreamReader(i2);
            foo = FileCopyUtils.copyToString(isr1);
            bar = FileCopyUtils.copyToString(isr2);
        }
        catch (IOException ioe) {
            //do something appropriate here
        } finally {
            if (i1 != null) i1.close();
            if (i2 != null) i2.close();
            if (isr1 != null) isr1.close();
            if (isr2 != null) isr2.close();
        }
    }

Is the first or second better? Is one faster over the other? Is it smart to close all of my streams even though it doesn't look at pretty?

Thanks for any insights (or edits).

Answer 1

The second you have lost its reference, but it looks more concise.

Yes, it is more concise. You're doing less with the object - importantly, you're not doing things you should be doing. Taking a shortcut will often produce less code, but by leaving file handles open pending the finalizer closing things, you'll find that you'll get exceptions which are hard to reproduce reliably.

Is there any memory difference between the two?

Maybe, but that's not relevant. The important resource here isn't the memory, but the file handle.

How about speed difference (in destruction)?

Again, likely to be irrelevant. Correctness is far more important.

(Note that your current second code is still not completely reliable - if one of the early close() calls throws an exception, you won't close the rest.)

If you're using Java 7, you should look at the try-with-resources statement, which makes all of this a lot simpler.

EDIT: As noted in JB Nizet's answer, the code you're using may already close the underlying stream - it depends on what FileCopyUtils is. If it's the Spring class, you're okay. If it's something else, you may not be. You should read the documentation. The general principle is as above though: you should make sure that something closes the streams. Don't assume that just because in this case you don't need to close things explicitly, that that's true in the general case.

Note that Guava's InputSupplier and OutputSupplier interfaces are useful here - it allows you to pass around the suppliers knowing that the input/output haven't been opened yet... which means that other code can perform the open/copy/close (or whatever is required) and handle it all for you... you don't end up using any objects in your code which could be closed.