5

Say I have the following code:

try {
    Reader reader = new FileReader("/my/file/location");
    //Read the file and do stuff
} catch(IOException e) {
    e.printStackTrace();
} finally {
    reader.close();
}

Why does reader only exist within the scope of the try brackets? Is this to stop an Exception being thrown earlier on in the code where the object reader was yet to be initialised or set?

How should I go about cleaning up objects that exist within the try clause, or do I have to bring them outside?

insidesin
  • 735
  • 2
  • 8
  • 26
  • 1
    Use a `try-with-resources` statement. – Sotirios Delimanolis Aug 13 '15 at 16:01
  • I still don't understand how I should be closing my `reader` object.. :/ I understand the similarity, but one is asking why and the other is asking 'why and how do I get around'. – insidesin Aug 13 '15 at 16:03
  • The scope of a local variable is (the rest of) its declaring block and try-catch-finally is no exception. The solution (albeit an ugly one) is to declare your variable in the block surrounding your try-catch. – biziclop Aug 13 '15 at 16:03
  • @insidesin - For your second question, see this prior question about try-with-resources: http://stackoverflow.com/questions/26516020/try-with-resources-vs-try-catch – Andy Thomas Aug 13 '15 at 16:07

1 Answers1

6

You have to bring them outside, because otherwise the variable exists only within the try block. But this is an improvement. If you change your code to this:

Reader reader = new FileReader("/my/file/location");
try {
    //Read the file and do stuff
} catch(IOException e) {
    e.printStackTrace();
} finally {
    reader.close();
}

then the code in the try-block, including the finally, can all rely on the reader having been created successfully. Otherwise, in your example, if the instantiation fails your code still tries to close the reader, which will still be null, on the way out.

In this changed example the exception thrown by the instantiation of the reader doesn't get handled by the catch block.

A combination of problems led you here, one is you're more worried about trying to squash all the exceptions with one catch block than you are about making sure the reader is in a valid state before you start calling methods on it. (You'll see this one-try-block-to-rule-them-all style a lot in JDBC code where people just can't bear to write all the nested try blocks, it's too ugly to tolerate.) Also, it's very toy-example-ish to catch all the exceptions locally like this, in real life if something goes wrong you want to let the exception be thrown so the rest of the application can know something went wrong, giving you something like

public void doStuffWithFile() throws IOException {
    Reader reader = new FileReader("/my/file/location");
    try {
        //Read the file and do stuff
    } finally {
        try {
            reader.close();
        } catch (IOException e) {
            // handle the exception so it doesn't mask
            // anything thrown within the try-block
            log.warn("file close failed", e);
        }
    }
}

which is very similar to what try-with-resources does. try-with-resources creates the objects first, then executes the try block, then closes the things it created, making sure that any exception thrown on close does not mask an exception thrown in the try-block. Creating the objects still has the potential to throw an exception, it doesn't get handled by the try-with-resources.

public void doStuffWithFile() throws IOException {
    try (Reader reader = new FileReader("/my/file/location")) {
        //Read the file and do stuff
    }
}
Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
  • 1
    You cannot (or it's pointless to) create the `FileReader` outside the `try` block. Change that to `Reader reader = null;` and then add a line `reader = new FileReader(...)` within the try block. Also, check for `null` in the `finally` block before closing. – Costi Ciudatu Aug 13 '15 at 16:53
  • one point of putting the reader outside of the try-block is so you don't have to check for null. hope the update makes more sense. – Nathan Hughes Aug 13 '15 at 16:55
  • Yep, it's clear now. The only problem is that you need to throw a checked exception from that method. But I can live with that, if it's for pedagogical purposes. – Costi Ciudatu Aug 13 '15 at 16:56
  • @Costi: ? i think i'm doing that, IOException is checked. see the second code example. edits are welcome, btw, if i'm not getting something. – Nathan Hughes Aug 13 '15 at 17:09
  • You are doing that, so it compiles fine; and I upvoted your answer, by the way. However, I would normally not throw an `IOException` from such a method if possible. Just log it if you can handle it or re-throw it as some `IllegalStateException`, but don't force me to catch (or re-throw) in the client code -- as I will most likely have no clue about what to do with it. But I think this argument doesn't really count for answering this particular question. :) – Costi Ciudatu Aug 13 '15 at 22:32
  • @Costi: that's a valid point, the checked exception can be a pain. – Nathan Hughes Aug 14 '15 at 00:39