Is there any preference for nested try / catch blocks?

Question

One of the things that always causes me problems with using Readers and Streams in Java is that the close() method can throw an exception. Since it is a good idea to put the close method in a finally block, this requires some inconvenient situation. I usually use this construct:

 FileReader fr = new FileReader("SomeFile.txt"); try { try { fr.read(); } finally { fr.close(); } } catch(Exception e) { // Do exception handling } 

But I also saw this construct:

 FileReader fr = new FileReader("SomeFile.txt"); try { fr.read() } catch (Exception e) { // Do exception handling } finally { try { fr.close(); } catch (Exception e) { // Do exception handling } } 

I prefer the first construct because there is only one catch block, and it seems more elegant. Is there a reason to prefer a second or alternative design?

UPDATE: would that change if I pointed out that both read and close only generate IOExceptions? Therefore, it seems to me that if the reading fails, then the failure will occur for the same reason.

Answer 1

I would always go for the first example.

If the exception were to be closed (in practice, this will never happen for FileReader), wouldn’t the standard method of processing be throwing an exception suitable for the caller? A closed exception will almost certainly surpass any problem you used the resource with. The second method is probably more suitable if your idea of ​​handling exceptions is to call System.err.println.

There is a question on how to exclude exceptions. ThreadDeath should always be returned, but any exception will ultimately stop this. Similarly, the error must exceed the RuntimeException and RuntimeException than the checked exceptions. If you really want you to be able to write code to follow these rules, then abstract it with the “run around” idiom.

Answer 2

I am afraid that in the first example there is a big problem, which is that if an exception occurs or after reading, the finally block is executed. So far, so good. But what if fr.close() raises another exception? This will “outperform” the first exception (a bit like placing return in a finally block) and you will lose all the information about what actually caused the problem .

Your finally block should use:

 IOUtil.closeSilently(fr); 

where is this utility method only:

 public static void closeSilently(Closeable c) { try { c.close(); } catch (Exception e) {} } 

Answer 3

I prefer the second. What for? If both read() and close() exceptions are thrown, one of them may be lost. In the first construct, the exception from close() cancels the exception from read() , and in the second, the exception from close() handled separately.

---

Starting with Java 7, the try-with-resources construct makes this a lot easier. read without concern for exceptions:

 try (FileReader fr = new FileReader("SomeFile.txt")) { fr.read(); // no need to close since the try-with-resources statement closes it automatically } 

With exception handling:

 try (FileReader fr = new FileReader("SomeFile.txt")) { fr.read(); // no need to close since the try-with-resources statement closes it automatically } catch (IOException e) { // Do exception handling log(e); // If this catch block is run, the FileReader has already been closed. // The exception could have come from either read() or close(); // if both threw exceptions (or if multiple resources were used and had to be closed) // then only one exception is thrown and the others are suppressed // but can still be retrieved: Throwable[] suppressed = e.getSuppressed(); // can be an empty array for (Throwable t : suppressed) { log(suppressed[t]); } } 

Only one attempt is required, and all exceptions can be safely handled. You can add a finally block if you want, but there is no need to close resources.

Answer 4

If both reads and closures throw an exception, the exception from the read will be hidden in option 1. Thus, the second option performs more error handling.

However, in most cases, the first option will still be preferable.

• In many cases, you cannot deal with exceptions in the method that they generated, but you should still encapsulate the processing of the stream as part of this operation.
• Try adding an entry to the code and see how the second approach is brought to this.

If you need to pass all the generated exceptions, this can be done .

Answer 5

The difference, as far as I see, is that there are different exceptions and reasons for the game at different levels, and

catch (exception e)

hides it. The only point on several levels is to distinguish between your exceptions and what you will do with them:

 try { try{ ... } catch(IOException e) { .. } } catch(Exception e) { // we could read, but now something else is broken ... } 

Answer 6

I usually do the following. First, define a template-based class to solve the try / catch mess problem

 import java.io.Closeable; import java.io.IOException; import java.util.LinkedList; import java.util.List; public abstract class AutoFileCloser { private static final Closeable NEW_FILE = new Closeable() { public void close() throws IOException { // do nothing } }; // the core action code that the implementer wants to run protected abstract void doWork() throws Throwable; // track a list of closeable thingies to close when finished private List<Closeable> closeables_ = new LinkedList<Closeable>(); // mark a new file protected void newFile() { closeables_.add(0, NEW_FILE); } // give the implementer a way to track things to close // assumes this is called in order for nested closeables, // inner-most to outer-most protected void watch(Closeable closeable) { closeables_.add(0, closeable); } public AutoFileCloser() { // a variable to track a "meaningful" exception, in case // a close() throws an exception Throwable pending = null; try { doWork(); // do the real work } catch (Throwable throwable) { pending = throwable; } finally { // close the watched streams boolean skip = false; for (Closeable closeable : closeables_) { if (closeable == NEW_FILE) { skip = false; } else if (!skip && closeable != null) { try { closeable.close(); // don't try to re-close nested closeables skip = true; } catch (Throwable throwable) { if (pending == null) { pending = throwable; } } } } // if we had a pending exception, rethrow it // this is necessary b/c the close can throw an // exception, which would remove the pending // status of any exception thrown in the try block if (pending != null) { if (pending instanceof RuntimeException) { throw (RuntimeException) pending; } else { throw new RuntimeException(pending); } } } } } 

Pay attention to the “pending” exception - this refers to the case when the exception created at the time of closure would mask the exception, which might really bother.

Finally, it first tries to close any decorated stream from the outside, so if you have a BufferedWriter that wraps FileWriter, we first try to close the BuffereredWriter, and if that fails, try closing the FileWriter itself.

You can use the above class as follows:

 try { // ... new AutoFileCloser() { @Override protected void doWork() throws Throwable { // declare variables for the readers and "watch" them FileReader fileReader = null; BufferedReader bufferedReader = null; watch(fileReader = new FileReader("somefile")); watch(bufferedReader = new BufferedReader(fileReader)); // ... do something with bufferedReader // if you need more than one reader or writer newFile(); // puts a flag in the FileWriter fileWriter = null; BufferedWriter bufferedWriter = null; watch(fileWriter = new FileWriter("someOtherFile")); watch(bufferedWriter = new BufferedWriter(fileWriter)); // ... do something with bufferedWriter } }; // .. other logic, maybe more AutoFileClosers } catch (RuntimeException e) { // report or log the exception } 

Using this approach, you never have to worry about trying / tricking / finally dealing with closing files.

If this is too heavy for your use, at least think about how to use the try / catch method and the "pending" variable approach that it uses.

Answer 7

The standard convention that I use is that you should not allow exceptions to the finally block.

This is because if the exception is already propagating, the exception that is excluded from the finally block will exceed the original exception (and therefore will be lost).

In 99% of cases, this is not what you want, since the initial exception is probably the source of your problem (any secondary exceptions may be side effects of the first, but they will hide your ability to find the source of the original exception and thus the real problem).

So your base code should look like this:

 try { // Code } // Exception handling finally { // Exception handling that is garanteed not to throw. try { // Exception handling that may throw. } // Optional Exception handling that should not throw finally() {} } 

Answer 8

Second approach.

Otherwise, I don’t see you catching an exception from the FileReader constructor

http://java.sun.com/j2se/1.5.0/docs/api/java/io/FileReader.html#FileReader(java.lang.String)

public FileReader (String fileName) throws a FileNotFoundException

So, I usually have a constructor inside the try block. the finally block checks to see if the reader is reading NOT before trying to close it.

The same applies to the data source, connection, Statement, ResultSet.

Answer 9

Sometimes nested try-catch is not preferred, consider this:

 try{ string s = File.Open("myfile").ReadToEnd(); // my file has a bunch of numbers // I want to get a total of the numbers int total = 0; foreach(string line in s.split("\r\n")){ try{ total += int.Parse(line); } catch{} } catch{} 

This is probably a bad example, but there are times when you need a nested try-cactch.

Answer 10

I like @Chris Marshall's approach, but I never like exceptions that swallow silently. I think it is best to log exceptions, especially if you disagree.

I always use the utility class to handle such general exceptions, but I would make it a little different from his answer.

I would always use logger (log4j for me) to log errors, etc.

 IOUtil.close(fr); 

A small modification of the utility method:

 public static void close(Closeable c) { try { c.close(); } catch (Exception e) { logger.error("An error occurred while closing. Continuing regardless", e); } } 

Answer 11

In some cases, a nested Try-Catch is inevitable. For example, when the error recovery code itself can cause and exclude. But to improve code readability, you can always extract a nested block in your own method. Check out this blog post for more examples on nested Try-Catch-finally blocks.