Java io ugly try-finally block
Solution 1
This is the correct idom (and it works fine):
InputStream in = null;
OutputStream out = null;
try {
in = new FileInputStream(inputFileName);
out = new FileOutputStream(outputFileName);
copy(in, out);
finally {
close(in);
close(out);
}
public static void close(Closeable c) {
if (c == null) return;
try {
c.close();
} catch (IOException e) {
//log the exception
}
}
The reason this works fine is that the exception thrown before you got to finally will be thrown after your finally code finishes, provided that your finally code doesn't itself throw an exception or otherwise terminate abnormally.
Edit: As of Java 7 (and Android SDK 19 - KitKat) there is now a Try with resources syntax to make this cleaner. How to deal with that is addressed in this question.
Solution 2
You could implement a utility method:
public final class IOUtil {
private IOUtil() {}
public static void closeQuietly(Closeable... closeables) {
for (Closeable c : closeables) {
if (c != null) try {
c.close();
} catch(Exception ex) {}
}
}
}
Then your code would be reduced to:
try {
copy(in, out);
} finally {
IOUtil.closeQuietly(in, out);
}
Additional
I imagine there'll be a method like this in a 3rd party open-source library. However, my preference is to avoid unnecessary library dependencies unless I'm using a large portion of its functionality. Hence I tend to implement simple utility methods like this myself.
Solution 3
try {
final InputStream in = new FileInputStream(inputFileName);
try {
final OutputStream out = new FileOutputStream(outputFileName);
try {
copy(in, out);
out.flush(); // Doesn't actually do anything in this specific case.
} finally {
out.close();
}
} finally {
in.close();
}
} catch (IOException exc) {
throw new SomeRelevantException(exc);
}
Remember that opening a stream may throw an exception, so you do need a try
between the stream openings (please don't do some hack involving null
s. Anything can throw an Error
(which are not an instances of Exception
).
It turns out that catch
and finally
should rarely share the same try
.
Since Java SE 7 you can write use try-with-resource to avoid so much indentation. It more or less does the same thing although there are suppressed exception hidden away.
try (
final InputStream in = new FileInputStream(inputFileName);
final OutputStream out = new FileOutputStream(outputFileName);
) {
copy(in, out);
out.flush(); // Doesn't actually do anything in this specific case.
} catch (IOException exc) {
throw new SomeRelevantException(exc);
}
You may want to use the Execute Around idiom.
I believe the standard good way to copy is using NIO's transferTo
/transferFrom
.
Solution 4
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 (initialize resources here) {
...
}
And they will be closed automatically after the block of code is finished. There is no need for the finally
part.
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!
Solution 5
Guava has very nice IO APIs that eliminate the need for this. For instance, your example would be:
Files.copy(new File(inputFileName), new File(outputFileName));
More generally, it uses the concept of InputSupplier
s and OutputSupplier
s to allow the InputStream
s and OutputStream
s to be created within its utility methods, allowing it full control over them so it can handle closing properly.
Additionally, it has Closeables.closeQuietly(Closeable)
which is basically the type of method most of the answers have suggested.
The IO stuff in it is still in beta and subject to change, but it's worth checking out and even using, depending on what it is you're working on.
Related videos on Youtube
The Student
Updated on October 05, 2020Comments
-
The Student over 3 years
Is there a not so ugly way of treat the
close()
exception to close both streams then:InputStream in = new FileInputStream(inputFileName); OutputStream out = new FileOutputStream(outputFileName); try { copy(in, out); } finally { try { in.close(); } catch (Exception e) { try { // event if in.close fails, need to close the out out.close(); } catch (Exception e2) {} throw e; // and throw the 'in' exception } } out.close(); }
update: All the above code is within one more try-catch, thanks for the warnings.
FINALLY (after the answers):
And a good utility method can be done using Execute Around idiom (thanks Tom Hawtin).
-
JRL about 14 yearsConsider catching exceptions at the proper level of abstraction, e.g. IOException, etc. instead of the too vague Exception.
-
The Student about 14 yearsI think that for this case, don't matter which Exception is thrown. Or why would?
-
meriton about 14 yearsPlease note that your code does not necessarily close
in
, see stackoverflow.com/questions/2441853/… -
Dave Jarvis about 8 yearsSee also: stackoverflow.com/questions/12552863/…
-
-
Sam Holder about 14 yearsthis is slightly different, as the in exception is not rethrown
-
Adam Paynter about 14 years+1, that's identical (character-for-character) to the answer I was typing! :)
-
Yishai about 14 yearsNice idea, a var-arg of closeables.
-
Adamski about 14 yearsActually I changed my mind as I don't agree with propagating the first Exception whilst ignoring subsequent ones. Better to suppress all exceptions given that this is simply relinquishing resources rather than an exceptional circumstance.
-
BalusC about 14 yearsI'd add a nullcheck. The closeable resource may not have been assigned. To the OP: the Commons IO library has similar methods: commons.apache.org/io/api-1.4/org/apache/commons/io/…
-
Brett about 14 yearsYou want to close the output stream "quietly"?!
-
Adamski about 14 yearsWhy have they left the IOUtils constructor public in Commons I/O? Also, this looks out of date as they haven't referenced Closeable but have overloaded the closeQuietly method with multiple types (Writer, Reader, etc).
-
Matthieu BROUILLARD about 14 years@Adamski: "I don't agree with propagating the first Exception while ignoring subsequent ones" Agree, should subclass IOException with some ChainedIOException containing all the failures if really an exception has to be raised.
-
OscarRyz about 14 years+1 I modify the code a little bit to use the if/try idiom I saw in a James Gosling code years ago.
-
eCommerce Guru about 14 yearsWhen it's finally released, this will be terrific. You can specify multiple resources in the try(...) parameters, which are closed cleany for you by the JVM.
-
The Student about 14 yearsWhy would the Java developers raise an exception if it is just to be ignored?
-
Yishai about 14 years@Tom Brito, although closing a stream is generally not an operation you want to get worked up about, logging the failure can be valuable. In some cases the correct closing is in fact what flushes the stream from a buffer and in fact things can go wrong there that are not "nothings." That is Tom Hawtin's point above.
-
The Student about 14 years@Yishai so you agree with my choose of the right answer (not this)
-
Yishai about 14 years@Tom Brito, since you chose my answer, I'm not going to argue ;-)
-
McDowell almost 14 yearsNote: this is not the correct idiom for all stream types. If the
OutputStream
buffers data (BufferedOutputStream
) or it writes closing blocks (ZipOutputStream
), you could lose data and the application would not handle it because it swallows exceptions. Logging is not a replacement for correct error handling. -
Yishai almost 14 years@McDowell, it is a good point regarding the output stream (see my comment to Adamski's answer that said something similar), however I would still say it is the standard idiom.
-
Brett over 13 yearsWhy would ignoring a failure to write out the end of the file be ok?
-
Brett over 13 yearsAlthough if the
BufferedReader
constructor fails (unlikely but does happen) you will leak. Also you are picking up the randomly confugured character encoding. -
Artem over 12 yearsBecause you know that it's a ByteArrayOutputStream?
-
Mr_and_Mrs_D about 11 yearsSince this is the accepted answer and people will look at it - and since you agree with @McDowell please address his comment in your answer - -in the catch block
-
Yishai about 11 years@Mr_and_Mrs_D, I don't know, with Java 6 end of life, the real answer is to use the Java 7 options, it certainly will be soon enough. I am loath to remake answers that got upvotes and acceptance on a different premise.
-
Mr_and_Mrs_D about 11 yearsRight - but much of what is going will keep being Java 6 - Android for one thing...
-
Mr_and_Mrs_D about 11 yearsVery nice actually - could you please comment on the benefits of it compared to the accepted answer ? Also if say
copy(in, out);
andout.close();
both throw won't we loose the exception thrown bycopy(in, out);
? -
Brett about 11 years@Mr_and_Mrs_D Kind of. In practice unbuffered I/O streams do not throw
IOException
fromclose
. Even if they did throwIOException
, the control flow would be the same. / Java SE 7 adds suppressed exceptions. I don't expect anyone to search through the suppressed tree to find the right exception, which isn't even really theoretically possible to do locally as there is not a defined ordering on exception priority. -
Mr_and_Mrs_D about 11 yearsThanks - still wouldn't the answer be more complete if you caught the close() exception and log it ?
-
Anton Savin over 9 yearsProbably you meant IOUtils.closeQuietly?
-
David Ehrmann almost 9 yearsBest answer here. You remembered to mention that it's a lot cleaner in Java 7, you don't have to remember that
close()
could also throw aRuntimeException
(or any otherThrowable
), and the pattern works for locks, too. -
Brett almost 9 years@DavidEhrmann To be fair, the question was asked over a year before Java SE 7 was released. (I added that in the May 3 '13 edit.)
-
djangofan over 8 yearsUsing this method Sonar says : "Possible null pointer dereference of inputStream" , so how do you think Sonar is suggesting this be rewritten?