Bad practice in JDK's try-with-resources example?

Question

I found this example in the try-with-resources documentation for Java:

static String readFirstLineFromFile(String path) throws IOException {
    try (BufferedReader br = new BufferedReader(new FileReader(path))) {
        return br.readLine();
    }
}

If the constructor for BufferedReader throws an exception, then the resources held by the FileReader won't be released. So isn't this a bad practice to write it like that instead of:

static String readFirstLineFromFile(String path) throws IOException {
    try (FileReader fr = new FileReader(path);
         BufferedReader br = new BufferedReader(fr)) {
        return br.readLine();
    }
}

This constructor of BufferedReadet doesn't throw any exception, as far as I know. — Dorian Gray, Nov 02 '17 at 20:18
@TobiasWeimer the second constructor does throw an IllegalArgumentException though. — f1sh, Nov 02 '17 at 20:24
I know that. But the first one doesn't, according to its documentation: https://docs.oracle.com/javase/9/docs/api/java/io/BufferedReader.html#BufferedReader-java.io.Reader- — Dorian Gray, Nov 02 '17 at 20:28
My opinion is that even if the specific constructor doesn't throw an exception, I still wouldn't use this as a teaching example as it brushes over a potential problem related to the subject. — biziclop, Nov 02 '17 at 20:47

sp00m · Answer 1 · 2017-11-02T20:32:53.290

Indeed, just gave it a quick try:

public class MyFileReader extends FileReader {

    public MyFileReader(String fileName) throws FileNotFoundException {
        super(fileName);
    }

    @Override
    public void close() throws IOException {
        System.out.println("Closing MyFileReader");
        super.close();
    }

}

public class MyBufferedReader extends BufferedReader {

    public MyBufferedReader(Reader in) {
        super(in);
        throw new RuntimeException();
    }

    @Override
    public void close() throws IOException {
        System.out.println("Closing MyBufferedReader");
        super.close();
    }

}

public String test(String path) throws IOException {
    try (BufferedReader br = new MyBufferedReader(new MyFileReader(path))) {
        return br.readLine();
    }
}

None of MyFileReader nor MyBufferedReader is closed... Good catch!

While with:

public String test(String path) throws IOException {
    try (FileReader fr = new MyFileReader(path); BufferedReader br = new MyBufferedReader(fr)) {
        return br.readLine();
    }
}

MyFileReader is closed.

BufferedReader constructor can indeed throw exceptions, see BufferedReader(Reader in, int sz) constructor (although not when coming from BufferedReader(Reader in) constructor, but the doc you linked should still alert on this possible issue IMHO).

Looks like you won the right to raise an issue :)

Adam Kotwasinski · Answer 2 · 2017-11-02T20:32:07.963

0

Unfortunately, you are right.

The below example shows this behaviour - instance of Internal is never closed.

public class Test {
    public static void main(String[] args) {
        try (External external = new External(new Internal())) {
        }
    }
}

class External implements Closeable {
    private Internal internal;
    public External(Internal internal) {
        this.internal = internal;
        throw new RuntimeException("boom");
    }

    @Override
    public void close() {
        System.out.println("External.close()");
        internal.close();
    }
}

class Internal implements Closeable {
    @Override
    public void close() {
        System.out.println("Internal.close()");
    }
}

edited Nov 02 '17 at 20:32

answered Nov 02 '17 at 20:20

Adam Kotwasinski

4,377
3
17
40

However in the concrete case, the constructor of BufferedReader doesn't throw any exception. – Dorian Gray Nov 02 '17 at 20:23
Any checked exception. – Adam Kotwasinski Nov 02 '17 at 20:24
Which exception does it throw exactly, and in which condition? The docunentation doesn't mention any exception to be thrown. https://docs.oracle.com/javase/9/docs/api/java/io/BufferedReader.html#BufferedReader-java.io.Reader- – Dorian Gray Nov 02 '17 at 20:25
@TobiasWeimer take the other constructor. https://docs.oracle.com/javase/9/docs/api/java/io/BufferedReader.html#BufferedReader-java.io.Reader-int- – f1sh Nov 02 '17 at 20:28
2

That other constructor is not used in the example. – Dorian Gray Nov 02 '17 at 20:29

Bad practice in JDK's try-with-resources example?

2 Answers2