commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Niall Pemberton (JIRA)" <j...@apache.org>
Subject [jira] Issue Comment Edited: (IO-249) Enhance closeQuietly to indicate success
Date Fri, 01 Oct 2010 05:09:33 GMT

    [ 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.


Mime
View raw message