You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Paul Benedict (JIRA)" <ji...@apache.org> on 2010/09/29 10:19:56 UTC
[jira] Created: (IO-249) Enhance closeQuietly to indicate success
Enhance closeQuietly to indicate success
----------------------------------------
Key: IO-249
URL: https://issues.apache.org/jira/browse/IO-249
Project: Commons IO
Issue Type: Improvement
Components: Utilities
Affects Versions: 2.0
Reporter: Paul Benedict
Assignee: Paul Benedict
Priority: Minor
A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916341#action_12916341 ]
Niall Pemberton commented on IO-249:
------------------------------------
See IO-157
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Sebb (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916736#action_12916736 ]
Sebb commented on IO-249:
-------------------------
Whis is "e" is in closeQuietly(e, Exception primary) ? Is it the Closable?
Perhaps I'm being dim, but I'm not sure what you expect the method to do.
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (IO-249) Enhance closeQuietly to
indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916783#action_12916783 ]
Niall Pemberton edited comment on IO-249 at 10/1/10 1:07 AM:
-------------------------------------------------------------
I was thinking about Brent's suggestion and came up with an alternative - a Closeable handler which does the closing - see attached example.
It has a number of features:
* Implements Closeable
* Can handle closing multiple Closeable objects
* Closeable objects can be added through the constructor
* Closeable objects can be added through an add() method that returns the Closeable - removes the need e.g. to define the Closeable outside the try/catch
* Provides an easy way to relate which Closeable's close() method threw an exception - which means e.g. logging can be improved
* Implements Iterable to provide an iterator of close exceptions
* provides a handle(Closeable, IOException) method that can be overriden to create custom (e.g. logging) implementations
* provides convenience methods to store and re-throw an original exception
So for example you could use it in the following way to log all close exceptions
{code}
public void copy(InputStream input, OutputStream output) throws IOException {
CloseableHandler handler = new CloseableHandler(input, output);
try {
IOUtils.copy(input, output);
} finally {
handler.close();
for (IOException ioe : handler) {
logger.log(Level.SEVERE, "Close error", ioe);
}
}
}
{code}
...or you could create a custom implementation
{code}
public static class LoggingCloseableHandler extends CloseableHandler {
private final Logger logger;
public LoggingCloseableHandler(Logger logger, Closeable... delegates) {
super(delegates);
this.logger = logger;
}
protected void handle(Closeable closeable, IOException e) {
logger.log(Level.SEVERE, "Error closing " + closeable, e);
}
}
{code}
...and then use that implementation:
{code}
public void copy(InputStream input, OutputStream output) throws IOException {
Closeable closeables = new LoggingCloseableHandler(logger, input, output);
try {
IOUtils.copy(input, output);
} finally {
closeables.close();
}
}
{code}
Another example showing that the InputStream & OutputStream don't have to be declared outside the try/catch. Also storing a caught exception - not used in this example - but it tells the handler whether its a *normal* close or an error has occurred.
{code}
public void copy(File fromFile, File toFile) throws IOException {
CloseableHandler handler = new CloseableHandler();
try {
InputStream input = handler.add(new FileInputStream(fromFile));
OutputStream output = handler.add(new FileOutputStream(toFile));
IOUtils.copy(input, output);
} catch(Exception e) {
handler.setCaughtException(e);
} finally {
handler.close();
for (IOException ioe : handler) {
logger.log(Level.SEVERE, "Close", ioe);
}
}
}
{code}
was (Author: niallp):
I was thinking about Brent's suggestion and came up with an alternative - a Closeable handler which does the closing - see attached example.
It has a number of features:
* Implements Closeable
* Can handle closing multiple Closeable objects
* Closeable objects can be added through the constructor
* Closeable objects can be added or an add() method that returns the Closeable - removes the need e.g. to define the Closeable outside the try/catch
* Provides an easy way to relate which Closeable's close() method threw an exception - which means e.g. logging can be improved
* Implements Iterable to provide and iterator of close exceptions
* provides a handle(Closeable, IOException) method that can be overriden to create custom (e.g. logging) implementations
* provides convenience methods to store and re-throw an original exception
So for example you could use it in the following way to log all close exceptions
{code}
public void copy(InputStream input, OutputStream output) throws IOException {
CloseableHandler handler = new CloseableHandler(input, output);
try {
IOUtils.copy(input, output);
} finally {
handler.close();
for (IOException ioe : handler) {
logger.log(Level.SEVERE, "Close error", ioe);
}
}
}
{code}
...or you could create a custom implementation
{code}
public static class LoggingCloseableHandler extends CloseableHandler {
private final Logger logger;
public LoggingCloseableHandler(Logger logger, Closeable... delegates) {
super(delegates);
this.logger = logger;
}
protected void handle(Closeable closeable, IOException e) {
logger.log(Level.SEVERE, "Error closing " + closeable, e);
}
}
{code}
...and then use that implementation:
{code}
public void copy(InputStream input, OutputStream output) throws IOException {
Closeable closeables = new LoggingCloseableHandler(logger, input, output);
try {
IOUtils.copy(input, output);
} finally {
closeables.close();
}
}
{code}
Another example showing that the InputStream & OutputStream don't have to be declared outside the try/catch. Also storing a caught exception - not used in this example - but it tells the handler whether its a *normal* close or an error has occurred.
{code}
public void copy(File fromFile, File toFile) throws IOException {
CloseableHandler handler = new CloseableHandler();
try {
InputStream input = handler.add(new FileInputStream(fromFile));
OutputStream output = handler.add(new FileOutputStream(toFile));
IOUtils.copy(input, output);
} catch(Exception e) {
handler.setCaughtException(e);
} finally {
handler.close();
for (IOException ioe : handler) {
logger.log(Level.SEVERE, "Close", ioe);
}
}
}
{code}
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
> Attachments: CloseableHandler.java
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Brent Worden (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916509#action_12916509 ]
Brent Worden commented on IO-249:
---------------------------------
What about adding the concept of an exception handler similar to what java.lang.Thread provides? IOUtils could provide a new closeQuielty(<T> closeable, IOExceptionHandler<T> handler) method whose implementation invokes a handler callback method upon receiving an IOException.
With this, users are free to implement IOExceptionHandler as they desire to satisfy any diagnostic needs they have.
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (IO-249) Enhance closeQuietly to
indicate success
Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916748#action_12916748 ]
Paul Benedict edited comment on IO-249 at 9/30/10 8:53 PM:
-----------------------------------------------------------
If you have access to the main exception, you could then record the suppressed exception from the Closeable:
{code}
public void closeQuietly(Closeable c, Exception e) {
try {
c.close();
} catch (Throwable t) {
if(jdk7 && e != null) {
e.addSuppressedException(t);
}
}
}
{code}
was (Author: paul4christ79):
If you have access to the main exception, you could then record the suppressed exception from the Closeable:
public void closeQuietly(Closeable c, Exception e) {
try {
c.close();
} catch (Throwable t) {
if(jdk7) {
e.addSuppressedException(t);
}
}
}
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916715#action_12916715 ]
Paul Benedict commented on IO-249:
----------------------------------
JDK 7 is introducing the notion of [suppressed exceptions|http://blogs.sun.com/darcy/entry/project_coin_updated_arm_spec]. These are exceptions that were not the cause of an exception, but were suppressed away because they shouldn't stop control flow. IOUtils does exactly this -- it's used commonly to close resources but the exceptions are unfortunately thrown away. All I am asking for is a way to get visibility to them without the complex try/catch block that closeQuietly() is intended to replace. If my current suggestion is not palatable, I then recommend the signature be: closeQuietly(e, Exception primary) so that suppression can be done.
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Gary Gregory (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916075#action_12916075 ]
Gary Gregory commented on IO-249:
---------------------------------
How about looking at this more generally? Should all "quiet" methods return a Throwable?
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (IO-249) Enhance closeQuietly to indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Niall Pemberton updated IO-249:
-------------------------------
Fix Version/s: 2.x
Like IO-157 changing the return type is binary incompatible with IO 1.4 - so this will need to wait until for a future version when we break binary compatibility
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Sebb (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916510#action_12916510 ]
Sebb commented on IO-249:
-------------------------
As I see it, the closeQuietly() method is intended to be used where one does not care about the success/failure, i.e. as a useful shorthand for
{code}
try (
in.close();
} catch (IOexception ieo) {
// ignored
}
{code}
So I agree with Neil here - if you want to get the Exception, use plain close().
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916332#action_12916332 ]
Paul Benedict commented on IO-249:
----------------------------------
Niall, can't this be done in 2.0?
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (IO-249) Enhance closeQuietly to indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Niall Pemberton updated IO-249:
-------------------------------
Attachment: IO-249-CloseableHandler.patch
Its probably irrelevant, but since I had an updated version that I did a while ago and I'm about to change machine - I thought I'd attach the latest CloseableHandler version here & test case so that it doesn't get lost
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 3.x
>
> Attachments: IO-249-CloseableHandler.patch
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Joerg Schaible (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916393#action_12916393 ]
Joerg Schaible commented on IO-249:
-----------------------------------
What about the closeLogged variant and adding commons-logging as optional dep?
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916748#action_12916748 ]
Paul Benedict commented on IO-249:
----------------------------------
If you have access to the main exception, you could then record the suppressed exception from the Closeable:
public void closeQuietly(Closeable c, Exception e) {
try {
c.close();
} catch (Throwable t) {
if(jdk7) {
e.addSuppressedException(t);
}
}
}
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (IO-249) Enhance closeQuietly to
indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916488#action_12916488 ]
Niall Pemberton edited comment on IO-249 at 9/30/10 10:40 AM:
--------------------------------------------------------------
@Joerg - I like the fact that Commons IO has no dependencies - that and with the popularity of things like SLF4J I really wouldn't want to add commons-logging as a dependency. Also with the introduction of the java.io.Closeable interface in JDK 1.5 then there is now only the need for one closeQueitly() method, rather than all the flavours Commons IO has - so anyone who wants their own custom impl - for example using their preferred logging framework - can easily do so.
@Paul - even though I disagreed with this for compatibility reasons - I actually don't like the proposal and would oppose it anyway. It just seems wrong to *return* an exception rather than throwing it, it encourages people to write strange code and the benefit diminishes since instead of replacing 4 lines with 1 line, you're now replacing 4 lines with 3:
So instead of having:
{code}
try {
input.close();
} catch (Exception ex) {
// do something here
}
{code}
You now have:
{code}
IOException ex = IOUtils.close(input);
if (ex != null) {
// do something here
}
{code}
IMO users would be better off doing it the traditional way with a try/catch rather than this
was (Author: niallp):
@Joerg - I like the fact that Commons IO has no dependencies - that and with the popularity of things like SLF4J I really wouldn't want to add commons-logging as a dependency. Also with the introduction of the java.io.Closeable interface in JDK 1.5 then there is now only the need for one closeQueitly() method, rather than all the flavours Commons IO has - so anyone who wants their own custom impl - for example using their preferred logging framework - can easily do so.
@Paul - even though I disagreed with this for compatibility reasons - I actually don't like the proposal and would oppose it anyway. It just seems wrong to *return* an exception rather than throwing it, it encourages people to write strange code and the benefit diminishes since instead of replacing 4 lines with 1 line, you're now replacing 4 lines with 3:
So instead of having:
{code}
try {
input.close();
} catch (Exception ex) {
// do something here
}
{code}
You now have:
{code}
IOException ex = IOUtils.close(input);
if (ex != null) {
// do something here
}
{code}
IMO users would be better off doing it the traditional way with a try/catch rather than this
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (IO-249) Enhance closeQuietly to
indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916488#action_12916488 ]
Niall Pemberton edited comment on IO-249 at 9/30/10 10:42 AM:
--------------------------------------------------------------
@Joerg - I like the fact that Commons IO has no dependencies - that and with the popularity of things like SLF4J I really wouldn't want to add commons-logging as a dependency. Also with the introduction of the java.io.Closeable interface in JDK 1.5 then there is now only the need for one closeQueitly() method, rather than all the flavours Commons IO has - so anyone who wants their own custom impl - for example using their preferred logging framework - can easily do so.
@Paul - even though I disagreed with this for compatibility reasons - I actually don't like the proposal and would oppose it anyway. It just seems wrong to *return* an exception rather than throwing it, it encourages people to write strange code and the benefit diminishes since instead of replacing 4 lines with 1 line, you're now replacing 4 lines with 3:
So instead of having:
{code}
try {
input.close();
} catch (Exception ex) {
// do something here
}
{code}
You now have:
{code}
IOException ex = IOUtils.close(input);
if (ex != null) {
// do something here
}
{code}
IMO users would be better off doing it the traditional way with a try/catch rather than this
was (Author: niallp):
@Joerg - I like the fact that Commons IO has no dependencies - that and with the popularity of things like SLF4J I really wouldn't want to add commons-logging as a dependency. Also with the introduction of the java.io.Closeable interface in JDK 1.5 then there is now only the need for one closeQueitly() method, rather than all the flavours Commons IO has - so anyone who wants their own custom impl - for example using their preferred logging framework - can easily do so.
@Paul - even though I disagreed with this for compatibility reasons - I actually don't like the proposal and would oppose it anyway. It just seems wrong to *return* an exception rather than throwing it, it encourages people to write strange code and the benefit diminishes since instead of replacing 4 lines with 1 line, you're now replacing 4 lines with 3:
So instead of having:
{code}
try {
input.close();
} catch (Exception ex) {
// do something here
}
{code}
You now have:
{code}
IOException ex = IOUtils.close(input);
if (ex != null) {
// do something here
}
{code}
IMO users would be better off doing it the traditional way with a try/catch rather than this
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916488#action_12916488 ]
Niall Pemberton commented on IO-249:
------------------------------------
@Joerg - I like the fact that Commons IO has no dependencies - that and with the popularity of things like SLF4J I really wouldn't want to add commons-logging as a dependency. Also with the introduction of the java.io.Closeable interface in JDK 1.5 then there is now only the need for one closeQueitly() method, rather than all the flavours Commons IO has - so anyone who wants their own custom impl - for example using their preferred logging framework - can easily do so.
@Paul - even though I disagreed with this for compatibility reasons - I actually don't like the proposal and would oppose it anyway. It just seems wrong to *return* an exception rather than throwing it, it encourages people to write strange code and the benefit diminishes since instead of replacing 4 lines with 1 line, you're now replacing 4 lines with 3:
So instead of having:
{code}
try {
input.close();
} catch (Exception ex) {
// do something here
}
{code}
You now have:
{code}
IOException ex = IOUtils.close(input);
if (ex != null) {
// do something here
}
{code}
IMO users would be better off doing it the traditional way with a try/catch rather than this
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (IO-249) Enhance closeQuietly to indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Niall Pemberton updated IO-249:
-------------------------------
Attachment: CloseableHandler.java
I was thinking about Brent's suggestion and came up with an alternative - a Closeable handler which does the closing - see attached example.
It has a number of features:
* Implements Closeable
* Can handle closing multiple Closeable objects
* Closeable objects can be added through the constructor
* Closeable objects can be added or an add() method that returns the Closeable - removes the need e.g. to define the Closeable outside the try/catch
* Provides an easy way to relate which Closeable's close() method threw an exception - which means e.g. logging can be improved
* Implements Iterable to provide and iterator of close exceptions
* provides a handle(Closeable, IOException) method that can be overriden to create custom (e.g. logging) implementations
* provides convenience methods to store and re-throw an original exception
So for example you could use it in the following way to log all close exceptions
{code}
public void copy(InputStream input, OutputStream output) throws IOException {
CloseableHandler handler = new CloseableHandler(input, output);
try {
IOUtils.copy(input, output);
} finally {
handler.close();
for (IOException ioe : handler) {
logger.log(Level.SEVERE, "Close error", ioe);
}
}
}
{code}
...or you could create a custom implementation
{code}
public static class LoggingCloseableHandler extends CloseableHandler {
private final Logger logger;
public LoggingCloseableHandler(Logger logger, Closeable... delegates) {
super(delegates);
this.logger = logger;
}
protected void handle(Closeable closeable, IOException e) {
logger.log(Level.SEVERE, "Error closing " + closeable, e);
}
}
{code}
...and then use that implementation:
{code}
public void copy(InputStream input, OutputStream output) throws IOException {
Closeable closeables = new LoggingCloseableHandler(logger, input, output);
try {
IOUtils.copy(input, output);
} finally {
closeables.close();
}
}
{code}
Another example showing that the InputStream & OutputStream don't have to be declared outside the try/catch. Also storing a caught exception - not used in this example - but it tells the handler whether its a *normal* close or an error has occurred.
{code}
public void copy(File fromFile, File toFile) throws IOException {
CloseableHandler handler = new CloseableHandler();
try {
InputStream input = handler.add(new FileInputStream(fromFile));
OutputStream output = handler.add(new FileOutputStream(toFile));
IOUtils.copy(input, output);
} catch(Exception e) {
handler.setCaughtException(e);
} finally {
handler.close();
for (IOException ioe : handler) {
logger.log(Level.SEVERE, "Close", ioe);
}
}
}
{code}
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
> Attachments: CloseableHandler.java
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916338#action_12916338 ]
Paul Benedict commented on IO-249:
----------------------------------
If the method currently returns void, there shouldn't be a problem with anyone's codebase once it returns a value. There wouldn't be compiled code today that would have a problem with this change. You disagree, I presume?
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916337#action_12916337 ]
Niall Pemberton commented on IO-249:
------------------------------------
Currently IO 2.0 is binary compatible with IO 1.4.
Because IO is widely used then if we break that compatibility - then we will have to change the package names to avoid that. I don't want to do that IO 2.0 - I want it to be a drop in replacement for IO 1.4
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Joerg Schaible (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916079#action_12916079 ]
Joerg Schaible commented on IO-249:
-----------------------------------
Alternatively provide methods:
{noformat}
IOUtils.closeLogged(<T> closeable); // Use internal logger
IOUtils.closeLogged(<T> closeable, Log logger);
{noformat}
We use this utility functions in our company everywhere instead of closeQuietly. However, this would add commons-logging as dependency.
Throwing out of closeQuietly will IMHO contradict the original purpose of the method.
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (IO-249) Enhance closeQuietly to indicate success
Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Niall Pemberton updated IO-249:
-------------------------------
Attachment: (was: CloseableHandler.java)
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 3.x
>
> Attachments: IO-249-CloseableHandler.patch
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917132#action_12917132 ]
Paul Benedict commented on IO-249:
----------------------------------
I definitely believe both of you are on the right track. The callback/handler strategy is a MUCH better alternative than my original suggestion. However, I would like to avoid instantiating a handler per resource. Is there an alternative? Otherwise, a callback will work since I can make it a singleton.
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
> Attachments: CloseableHandler.java
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (IO-249) Enhance closeQuietly to indicate success
Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/IO-249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917235#action_12917235 ]
Paul Benedict commented on IO-249:
----------------------------------
On second thought, I think the handler idea is great. If possible, I would like to write the implementation for 2.0. Since it is a new method, it won't affect compatiblity.
> Enhance closeQuietly to indicate success
> ----------------------------------------
>
> Key: IO-249
> URL: https://issues.apache.org/jira/browse/IO-249
> Project: Commons IO
> Issue Type: Improvement
> Components: Utilities
> Affects Versions: 2.0
> Reporter: Paul Benedict
> Assignee: Paul Benedict
> Priority: Minor
> Fix For: 2.x
>
> Attachments: CloseableHandler.java
>
>
> A convention of some programmers is to emit a log warning when a resource fails to close. Granted, such a condition is an error, but there's no reasonable recourse to the failure. Using IOUtils.closeQuietly() is very useful but all information about the success/failure is hidden. Returning Throwable will give insight into the error for diagnostic purposes. This change will be compatible with today's usage since the method currently returns void.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.