You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Al McDowell (JIRA)" <ji...@apache.org> on 2010/08/08 15:35:20 UTC
[jira] Created: (IO-247) IOUtils.closeQuietly should be documented
or deprecated
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.
[jira] Resolved: (IO-247) IOUtils.closeQuietly should be documented
or deprecated
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-247?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Niall Pemberton resolved IO-247.
--------------------------------
Assignee: Niall Pemberton
Fix Version/s: 2.0
Resolution: Fixed
Examples added:
http://svn.apache.org/viewvc?view=revision&revision=995152
> 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
> Assignee: Niall Pemberton
> Fix For: 2.0
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> 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.
[jira] Commented: (IO-247) IOUtils.closeQuietly should be
documented or deprecated
Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-247?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12896767#action_12896767 ]
Henri Yandell commented on IO-247:
----------------------------------
Good idea.
> 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
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> 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.