If you need to combine multiple Try / Catch blocks in a method

Question

Looking at my method, I'm not sure if I need to use three separate try / catch blocks. Should I use a single method for the whole method? What is good practice?

public void save(Object object, File file) { BufferedWriter writter = null; try { writter = new BufferedWriter(new FileWriter(file)); } catch (IOException e) { e.printStackTrace(); } Dictionary dictionary = (Dictionary)object; ArrayList<Card> cardList = dictionary.getCardList(); for (Card card: cardList) { String line = card.getForeignWord() + " / " + card.getNativeWord(); try { writter.write(line); } catch (IOException e) { e.printStackTrace(); } } try { writter.flush(); writter.close(); } catch (IOException e) { e.printStackTrace(); } } 

Answer 1

Of course, you do not want the three separate blocks to be with the code as it is. In the first block, you find an error when setting up writer , but then in subsequent blocks you use writer , which does not make sense if the first block fails. As a result, you will get a NullPointerException when an I / O error occurs - not perfect .:-)

There's a lot of room for style in this stuff, but here's a pretty standard reinterpretation of your function. It uses only two blocks (although you can add the third part, see comments in the code):

 public void save(Object object, File file) { BufferedWriter writter = null; try { writter = new BufferedWriter(new FileWriter(file)); Dictionary dictionary = (Dictionary)object; ArrayList<Card> cardList = dictionary.getCardList(); for (Card card: cardList) { String line = card.getForeignWord() + " / " + card.getNativeWord(); writter.write(line); // <== I removed the block around this, on // the assumption that if writing one card fails, // you want the whole operation to fail. If you // just want to ignore it, you would put back // the block. } writter.flush(); // <== This is unnecessary, `close` will flush writter.close(); writter = null; // <== null `writter` when done with it, as a flag } catch (IOException e) { e.printStackTrace(); // <== Usually want to do something more useful with this } finally { // Handle the case where something failed that you *didn't* catch if (writter != null) { try { writter.close(); writter = null; } catch (Exception e2) { } } } } 

Note in the finally block: here you can handle the usual case (in this case the writter will be null ), or you can handle the exception that you did not catch (this isn’t unusual, one of the main points of the exceptions is handling what suits the this level, and transfer anything else before the caller). If writter is !null , close it. And when you close it, eat any exception that happens, or you close the original. (I have utility functions for closing things while using an exception, for this situation. For me it could be writter = Utils.silentClose(writter); [ silentClose always returns null ]). Now in this code you can not expect other exceptions, but A) you can change this later, and B) a RuntimeException can occur at any point. It’s best to get used to using the template.

Answer 2

This is a matter of taste - there is no right answer.

Personally, I prefer one try / catch block. If I see a lot of them, I start to worry that my method does too much and becomes too large. This is a sign that it is time to break it into smaller methods.

This is how I write this method:

 public void save(Object object, File file) throws IOException { BufferedWriter writer = null; try { writer = new BufferedWriter(new FileWriter(file)); Dictionary dictionary = (Dictionary) object; ArrayList<Card> cardList = dictionary.getCardList(); for (Card card : cardList) { String line = card.getForeignWord() + " / " + card.getNativeWord(); writer.write(line); } } finally { IOUtils.close(writer); } } 

I will have an IOUtils class that encapsulates the proper closure of threads, readers and writers. You will write this code again and again. I would make it one liner as a static method in the class - or use the Apache Commons IO Utils JAR JAR if I would not mind another dependency.

Answer 3

Which is good depends on what your code should do. For example, the second try / catch is inside the loop, which means that it will process all card in the list, even if processing of one of them failed. If you use one try/catch for the whole method, this behavior will be changed. Use one try/catch if allowed to use it.

Answer 4

With try-catch, there is more to “not with” than to “doing”, I'm afraid ...

This document lists some of them:

http://today.java.net/article/2006/04/04/exception-handling-antipatterns

However, there is something I found it interesting:

http://www.wikijava.org/wiki/10_best_practices_with_Exceptions

Answer 5

In the specific case of I / O, I suggest you familiarize yourself with some of the available shared libraries, such as Guava and Jakarta. They have some pretty good scripts for doing closed closures.

eg. Commons IO IOUtils

They can clear your code a bit.

Answer 6

I prefer to write two try blocks, one for finally and one for catch. Due to the way it is written, you never set the author to null, and you do not need to check for null when closing. The only problem is the exception, while closing will hide the exception during processing, but this is only the problem if you want to handle the two exceptions differently.

 public void save(Object object, File file) { try { BufferedWriter writter = new BufferedWriter(new FileWriter(file)); try { Dictionary dictionary = (Dictionary)object; ArrayList<Card> cardList = dictionary.getCardList(); for (Card card: cardList) { String line = card.getForeignWord() + " / " + card.getNativeWord(); writter.write(line); } } finally { writter.flush(); writter.close(); } } catch (IOException e) { e.printStackTrace(); //Or something more useful } }