Java code style for open stream try/finally block [duplicate]

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!

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.

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.

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.

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.

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.

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.

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.