Should multiple Try/Catch blocks in a method be combined

Question

Looking on my method I'm not sure I have to use three separate try/catch blocks. Should I use the only one for the whole method? What is a 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

You certainly don't want three separate blocks with the code as it is. In the first block, you're catching an error setting up writer, but then in subsequent blocks you're using writer, which doesn't make sense if the first block failed. You'll end up throwing a NullPointerException when an I/O error occurs — not ideal. :-)

There's a lot of room for style in this stuff, but here's a fairly standard reinterpretation of your function. It only uses two blocks (though you may choose to add back a third; see the 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 on the finally block: Here, you may be handling the normal case (in which case writter will be null), or you might be handling an exception you didn't catch (which isn't unusual, one of the main points of exceptions is to handle what's appropriate at this level and to pass anything else up to the caller). If writter is !null, close it. And when you close it, eat any exception that occurs or you'll obscure the original one. (I have utility functions for closing things whilst eating an exception, for exactly this situation. For me, that could would be writter = Utils.silentClose(writter); [silentClose always returns null]). Now, in this code you may not be expecting other exceptions, but A) You may change that later, and B) RuntimeExceptions can occur at any point. Best to get used to using the pattern.