IOUtils.closeQuietly should be documented or deprecated
-------------------------------------------------------
Key: IO-247
URL: https://issues.apache.org/jira/browse/IO-247
Project: Commons IO
Issue Type: Improvement
Components: Utilities
Affects Versions: 1.4
Environment: All
Reporter: Al McDowell
This is primarily a documentation and education issue.
{{IOUtils.closeQuietly}} does exactly what is says on the tin, so the method
isn't defective, but I frequently see it abused to write buggy code of this
form:
{code:title=BadStreamHandling.java|borderStyle=solid}
//don't write code like this
byte[] data = "Hello, World".getBytes();
OutputStream out = null;
try {
File temp = File.createTempFile(IOBug.class.getName(), ".txt");
System.out.println(temp);
out = new FileOutputStream(temp);
out = new BufferedOutputStream(out);
out.write(data);
} catch (IOException e) {
// error handling
} finally {
//errors writing buffered output are swallowed
IOUtils.closeQuietly(out);
}
{code}
Search a site like
[stackoverflow.com|http://stackoverflow.com/search?q=%22IOUtils.closeQuietly%22]
for {{IOUtils.closeQuietly}} and half the examples are broken in some way. The
contract for most closeable classes is that the close method can throw an
{{IOException}}. By swallowing the exception, callers are not handling errors
and this is rarely acceptable. In the case of output streams, this can lead to
data loss.
The value of a utility library is diminished if it just provides a faster way
to write bad code.
*Proposed fix*
I'd like to see some advice added to the javadoc about how and when to use
these methods - canonical examples wouldn't hurt.
This example code would honour the class contract:
{code:title=BetterStreamHandling.java|borderStyle=solid}
//better
byte[] data = "Hello, World".getBytes();
OutputStream out = null;
try {
File temp = File.createTempFile(IOBug.class.getName(), ".txt");
System.out.println(temp);
out = new FileOutputStream(temp);
out = new BufferedOutputStream(out);
out.write(data);
out.close(); //close errors are handled
} catch (IOException e) {
// error handling
} finally {
IOUtils.closeQuietly(out);
}
{code}
Note: using {{flush}} is not an acceptable alternative. It is better to write
to the abstraction and not the implementation and some stream types will not be
able to commit all data until {{close}} is called anyway. _I don't mean to
lecture - I'm just trying to be thorough - this is another common piece of
advice used in connection with these methods._
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
