Java code style for open stream try/finally block

Question

This is a code style question. I notice much example code including some examples from Oracle ensure that a stream is closed in the following manner:

InputStream in = null;
try {
    in = acquireStream();
    ...
} finally {
    if (in != null) in.close();
}

Note the initialization to null and check for null in the finally block.

I tend to write code like this:

InputStream in = acquireStream();
try {
    ...
} finally {
    in.close();
}

Are there advantages or disadvantages to either approach? I like my style because I don't need the null check. Also I like to avoid null when possible. But since the Oracle style is so common in online examples, I'm wondering if mine has some hidden error.

I ask the same question for InputStream, OutputStream, java.sql.Connection, java.sql.PreparedStatement, etc. I tend to acquired the resource outside the try block and then close it in finally without a null check. Is there anything I am missing other than stylistic differences?

Thanks.

darijan · Answer 1 · 2016-09-25T10:50:22.173

Since Java 7 there is a much nicer way to write try-finally block in regards to Closeable resources.

Now you can create your resources in parenthesis after the try keyword, like this:

try (init resources) {
   ...
}

And they will be closed automcatically after the block of code is finished. There is no need to close the streams in finally block.

An example:

try (
   ZipFile zf = new ZipFile(zipFileName);
   BufferedWriter writer = Files.newBufferedWriter(outputFilePath, charset);
) {
    // Enumerate each entry
    for (Enumeration entries = zf.entries(); entries.hasMoreElements();) {
        // Get the entry name and write it to the output file
        String newLine = System.getProperty("line.separator");
        String zipEntryName = ((java.util.zip.ZipEntry)entries.nextElement()).getName() + newLine;
        writer.write(zipEntryName, 0, zipEntryName.length());
    }
}

And after the for loop is done, the resources will be closed!

ILMTitan · Answer 2 · 2011-01-18T22:31:21.663

The answer is, no, there is no hidden error with doing it your way. It is purely a style thing. I typically never have a try catch finally block, only try catch blocks and try finally blocks.

They tend to end up looking like this:

try {
    InputStream in = acquireStream();
    try {
        ...
    } finally {
        in.close();
    }
} catch (IOException e) {
    ... handle exception
}

There is no reason to put acquireStream() in the try finally block. If in is never assigned to a valid stream, you can never close it. An explicit null check is totally unnecessary. Additionally, it is rare that you want to handle an exception on the close() differently than an exception in the main processing block.

score 3 · Answer 3 · answered Jan 18 '11 at 18:10

3

Typically what you have is try, catch, finally. In that case, it's advantageous to use the standard SUN approach because you can capture any errors that occured in acquireStream() inside of the try, catch.

answered Jan 18 '11 at 18:10

Amir Afghani

37,814
16
84
124

score 3 · Answer 4 · answered Jan 18 '11 at 18:16

I would use

 InputStream in = null;
 try {
     in = acquireStream();
     ...
 } finally {
     if (in != null) in.close();
 }

if aquireStream() throws any checked exception and I am planning to handle it.

Else, I will use this

InputStream in = acquireStream();
try {
     ...
} finally {
     in.close();
}

on NPE:

I would rather let NPE to propagate and not handle any run-time exception.

score 2 · Answer 5 · answered Jan 18 '11 at 18:17

2

I think it's safer to acquire the stream within the try block.

There's another option for closing - instead of checking for null you can do the following:

finally {
    IOUtils.closeQuietly(in);
}

This does require Apache Commons-IO to do this, but it will do the null check for you. It's also a nice way stylistically to do this.

answered Jan 18 '11 at 18:17

Jonathan Holloway

62,090
32
125
150

See: http://commons.apache.org/io/api-release/org/apache/commons/io/IOUtils.html#closeQuietly(java.io.InputStream) – Jonathan Holloway Jan 18 '11 at 18:21

score 0 · Answer 6 · answered Jan 18 '11 at 18:13

0

If your acquireStream() returned null, You will get NPE when you will try to close your stream in finally block and it will be uncaught.

answered Jan 18 '11 at 18:13

jjczopek

3,319
2
32
72

score 0 · Answer 7 · answered Jan 18 '11 at 18:14

0

I favor the first. Some I/O operators require they are inside a try/catch for some condition or other. Also, operations can always return null unexpectedly, even though that is not in the manual.

answered Jan 18 '11 at 18:14

ddyer

1,792
19
26

score 0 · Answer 8 · edited May 23 '17 at 11:47

0

I usually do this:

InputStream in = null;
try { 
  in = acquire();
  ...
} finally { 
   if( in != null ) try {
       in.close();
    } catch( IOException ioe ) {
       // ignore exception while closing
    }
}

While closing the resource, an exception may be thrown, in which case you'll need an extra try/catch , but most of the times it's ok for me to ignore it ( I'm closing it after all ) but this is the only place where I use an if without braces.

I saw this from Huckster code long ago.

edited May 23 '17 at 11:47

Community

1
1

answered Jan 18 '11 at 18:36

OscarRyz

196,001
113
385
569

Does actually compile (whilst ignoring a declared, if improbable in this case, exception). – Tom Hawtin - tackline Jan 18 '11 at 19:32
Referencing of `in` which is not definitely assigned in the `finally` clause. – Tom Hawtin - tackline Jan 18 '11 at 21:21
@Tom, you're absolutely right. I got confused with the : `Object o;if( cond ) { o = something; } else { o = somethingelse; }` idiom. Fixed! – OscarRyz Jan 18 '11 at 21:36

Java code style for open stream try/finally block

8 Answers8