garydgregory commented on code in PR #857:
URL: https://github.com/apache/commons-io/pull/857#discussion_r3404035142


##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -801,10 +805,12 @@ private static void closeQ(final Closeable closeable) {
      *
      * <pre>
      * try {
-     *     return IOUtils.copy(inputStream, outputStream);
+     *     IOUtils.copy(inputStream, outputStream);
+     *     outputStream.close(); // close errors are handled
+     *     outputStream = null;
      * } finally {
      *     IOUtils.closeQuietly(inputStream);
-     *     IOUtils.closeQuietly(outputStream);
+     *     IOUtils.closeQuietly(outputStream); // only if normal close was 
skipped

Review Comment:
   @Sarankumar18 
   This example makes no sense to me:
   - First, you `close()` the OS on line 809 with the contradictory comment 
"close errors are handled"; the close() call _does not_ handle errors.
   - Second, in the finally block, the comment "only if normal close was 
skipped" is not true because the finally block is always executed.
   Please come up with an example and comments that make sense and are 
consistent with what actually happens.
   TY



##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -853,9 +859,11 @@ public static void closeQuietly(final Closeable closeable) 
{
      *
      * <pre>
      * try {
-     *     return IOUtils.copy(inputStream, outputStream);
+     *     IOUtils.copy(inputStream, outputStream);
+     *     outputStream.close(); // close errors are handled

Review Comment:
   Same comment as above.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to