com.google.common.io.Closeables implementation pointless

[GoogleCodeExporter]

This class could be useful if it wasn't so useless. 

The method close(Closeable, boolean) - why there is a CHECKED exception 
declared in the method signature? If I wanted to catch and use this exception, 
I wouldn't use this method. The whole point of it is closing a closeable 
without caring about exception. The method should be public void 
closeAll(Closeable... values), no checked exception in the method signature!

And why there are separate methods for another two Closeable subclasses?? They 
are doing the same thing, just change the signature to the one above and you'll 
have just one useful method in place of 0.5 useful and 2.5 useless.

Original issue reported on code.google.com by Michael....@gmail.com on 25 Jul 2014 at 9:42

[GoogleCodeExporter]

I can make the changes, just give me access.

Original comment by Michael....@gmail.com on 25 Jul 2014 at 9:43

[GoogleCodeExporter]

We've attempted to address both questions in the Javadoc. Let us know if we can 
clarify either answer.

"""
This is primarily useful in a finally block, where a thrown exception needs to 
be logged but not propagated (otherwise the original exception will be lost)....
Example:

  public void useStreamNicely() throws IOException {
    SomeStream stream = new SomeStream("foo");
    boolean threw = true;
    try {
      // ... code which does something with the stream ...
      threw = false;
    } finally {
      // If an exception occurs, rethrow it only if threw==false:
      Closeables.close(stream, threw);
    }
  }
"""

"With a writable resource, a failure thrown from close() should be considered 
as significant as a failure thrown from a write method because it may indicate 
a failure to flush bytes to the underlying resource.... While it's not safe in 
the general case to ignore exceptions that are thrown when closing an I/O 
resource, it should generally be safe in the case of a resource that's being 
used only for reading, such as an InputStream. Unlike with writable resources, 
there's no chance that a failure that occurs when closing the stream indicates 
a meaningful problem such as a failure to flush all bytes to the underlying 
resource."

Original comment by cpov...@google.com on 25 Jul 2014 at 1:54

• Changed state: WorkingAsIntended

[GoogleCodeExporter]

and how is it different from stream.close()???????

There is still the same checked exception vented out!!! 

Oh, never mind, last thing I need is someone going defensive about such a small 
thing. I am maintaining a utility project that fixes all those bloopers in 
guava, I call it "guavax", like in this case:

public void closeAll(Closeable... closeables) {
   for(final Closeable c: closeables) {
      try {
          c.close();
      }
      catch(IOException x) {
         // nothing to do
      }
   }
}

then in the finally or not, I can closeAll(stream, socket, reader) and what not.

Original comment by Michael....@gmail.com on 25 Jul 2014 at 4:58

[GoogleCodeExporter]

actually, in my library it's 

if(c != null) c.close().

That takes care of the references that had not been assigned yet, which is 
typical if error strikes. Again, I am not going to spend any more time on this 
- if you understand the problem and the solution, please take it. If you don't 
I'll maintain the fixes on my side as usual. 

Original comment by Michael....@gmail.com on 25 Jul 2014 at 5:00

[GoogleCodeExporter]

The difference from stream.close() is that an exception from Closeables.close 
won't override a previous exception. As a result, Closeables.close is useful 
only when used with the try-finally recipe above.

As for closeAll() and other methods that prevent any IOException from being 
thrown, the danger is in code like this:

stream.write("foo");
closeAll(stream);

If stream.close() throws an exception, there is no guarantee that "foo" was 
ever written to the underlying file, even if write("foo") didn't throw an 
exception.

Original comment by cpov...@google.com on 25 Jul 2014 at 5:03

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:08

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07

• Added labels: Restrict-AddIssueComment-Commit