Is it okay to use try catch inside finally?

Question

Hi,

I am using a buffered writer and my code, closes the writer in the finally block. My code is like this.

 ...........
    BufferedWriter theBufferedWriter = null;
    try{
    theBufferedWriter =.....
    ....
    ......
    .....
    } catch (IOException anException) {
    ....
    } finally {
        try {
            theBufferedWriter.close();              
        } catch (IOException anException) {
            anException.printStackTrace();
            }
    }

I have to use the try catch inside the clean up code in finally as theBufferedWriter might also throw an IOException. I do not want to throw this exception to the calling methos. Is it a good practice to use a try catch in finally? If not what is the alternative? Please suggest.

Regards, Hiral

Answer 1

Absolutely. I would say that the way you have written it is the way to go.

The only remark I have is the usual one; don't ignore exceptions.

I agree that it looks a bit clunky and there is indeed an proposal for improvement for this kind of situations: Automatic Resource Management Blocks, Joshua Bloch.

Answer 2

A somewhat nicer way to do this is to use IOUtils.closeQuiety from Apache commons-io. It keeps your code tidy and eliminates some of the boilerplate that's inherent in Java.

You code then becomes:

BufferedWriter theBufferedWriter = null;
try{
    theBufferedWriter = ...
    ...
} catch (IOException anException) {
    ...
} finally {
    IOUtils.closeQuietly(theBufferedWriter);
}

Much nicer and more expressive.

Answer 3

This is what we will have to live with until Java 7 and ARM Blocks.

Answer 4

Or you can use Lombok and the @Cleanup annotation and you shall never write a try catch inside finally again.

This is how you would normally write it (Note the throws IOException):

//Vanilly Java

import java.io.*;

public class CleanupExample {
  public static void main(String[] args) throws IOException {
    InputStream in = new FileInputStream(args[0]);
    try {
      OutputStream out = new FileOutputStream(args[1]);
      try {
        byte[] b = new byte[10000];
        while (true) {
          int r = in.read(b);
          if (r == -1) break;
          out.write(b, 0, r);
        }
      } finally {
        out.close();
      }
    } finally {
      in.close();
    }
  }
}

Now with Lombok you just write @Cleanup on the streams

import lombok.Cleanup;
import java.io.*;

 public class CleanupExample {
   public static void main(String[] args) throws IOException {
     @Cleanup InputStream in = new FileInputStream(args[0]);
     @Cleanup OutputStream out = new FileOutputStream(args[1]);
     byte[] b = new byte[10000];
     while (true) {
       int r = in.read(b);
       if (r == -1) break;
       out.write(b, 0, r);
     }
   }
 }

Answer 5

It's OK but you should test if theBufferedWriter is not null before closing it.
You could also do:

BufferedWriter theBufferedWriter;
try {
    theBufferedWriter = new ...
    try {
        ...
    } finally {
        try {
            theBufferedWriter.close();
        } catch (IOException closeException) {
            closeException.printStackTrace();
        }
    }
} catch (IOException anException) {
    ...
}

or:

BufferedWriter theBufferedWriter;
try {
    theBufferedWriter = new ...
} catch (IOException createException) {
    // do something with createException 
    return;  // assuming we are in a method returning void
}

try {
    ...
} catch (IOException anException) {
    ...
    // assuming we don't return here
}

try {
    theBufferedWriter.close();
} catch (IOException closeException) {
    closeException.printStackTrace();
}

but mostly I do such operations (e.g. writing a file) in a dedicated method and prefer to throw the/an Exception so the caller can handle it (e.g. asking for another file, stopping the application, ...):

void someMethod(...) throws IOException {
    BufferedWriter theBufferedWriter = new ...

    try {
        ...
    } catch (IOExcepption anException) {
        try {
            theBufferedWriter.close();
        } catch (IOException closeException) {
            closeException.printStackTrace();
            // closeException is not thrown, anException represents the main/first problem
        }
        throw anException;
    }

    theBufferedWriter.close();  //  throws the Exception, if any
}

_{Please note: English is not my first nor my second language, any help would be appreciated}

Answer 6

It's ok to put a try-catch in a finally. It is the tool that does what you want to do. However, I feel the thrown IOException on close is uncommon enough that I would allow it to suppress any exception in the body like so.

try {
    BufferedWriter writer = .....
    try {
        .....
    } finally {
       writer.close();
    }
 } catch (IOException e) {
     ....
 }