Is it good practice to try-with-resource a file writer

Question

I saw this example on the web and in the Book "Effective Java" (by Joshua Bloch).

try(BufferedWriter writer = new BufferedWriter(new FileWriter(fileName))){
  writer.write(str); // do something with the file we've opened
}
catch(IOException e){
  // handle the exception
}

There is no problem with this example, BufferedWriter which will automatically get closed and that, in turn, will close the FileWriter; however, in other case, if we declare 2 nested resources this way:

try (AutoClosable res = new Impl2(new Impl1())) {... }

I guess it might happen that new Impl1() performs fine, but new Impl2() crashes, and in this case Java would have no reference to the Impl1, in order to close it.

Shouldn't it be a better practice to always declare multiple resources independently (even if not required in this case), like this?

try(FileWriter fw = new FileWriter(fileName); 
    BufferedWriter writer = new BufferedWriter(fw)){ ... }

Answer 1

First of all, for the sake of a little sanity check, I would say, that I could not find the example, you provided, in Joshua Bloch's Effective Java (3rd Edition, 2018). If you are reading the previous version, it should be better to get the latest one. If it is my bad, please refer particular page number to me.

---

Now with respect to the question itself.

Let's begin with JSL §14.20.3, which says, that Resources are separated by ;. This means, that disregarding of how long our decoration chain of the object creations will be (e.g. new Obj1(new Obj2(...new ObjK(..))); it will be treated as a one single resource, as it is a one definition/statement.

As we now know what does a Single Resource constitute, let me shed some light from my observation.

Reasons, why it is better to define resources separately

1. JSL §14.20.3 also states, that:

If a resource fails to initialize (that is, its initializer expression throws an exception), then all resources initialized so far by the try-with-resources statement are closed. If all resources initialize successfully, the try block executes as normal and then all non-null resources of the try-with-resources statement are closed.

Q: What does that mean for us?

A: If that single resource is a chain of objects passed into wrapping constructors, it does not mean, that throwing an exception from the initialization phase will close those embedded resources, rather the .close() method will be invoked on the parent (wrapper, enclosing) objects, and therefore, you might easily end up with a resource leak.

On the other hand, you can be resting assured that all resources will be closed, if you have defined them separately.

2. JSL §14.20.3 also states, that:

An exception from the closing of one resource does not prevent the closing of other resources. Such an exception is suppressed if an exception was thrown previously by an initializer, the try block, or the closing of a resource.

Q: What does that mean for us?

A: If you have declared your resources separately, then it does not matter which throws exception and which does not. They all will be closed successfully.

3. Unfortunately the book you mentioned (at least the 3rd version it) does not cover the question you brought up here; however, I did some more research and found that Core Java (10th Edition), by Cay S. Horstmann, confirms the point I referred to above, in its §7.2.5:

When the block exits normally, or when there was an exception, the in.close() method is called, exactly as if you had used a finally block.

and

No matter how the block exits, both in and out are closed.

_{in and out are Autoclosable objects in the examples given in the book.}

Q: What does this mean?

A: It means, that exceptions thrown from any phase of one of the resources, does not anyhow affect how another resources are closed.

---

Based on all above, I think, that it also depends on how the resources are implemented. E.g. if the enclosing resource, upon an invocation of its .close(), implements the logic to close the enclosed resource, then your concern:

I guess it might happen that new Impl1() performs fine, but new Impl2() crashes, and in this case Java would have no reference to the Impl1, in order to close it.

will not really be a problem in your particular case, as the container resource which is being closed, will hold a reference to the contained resource and eventually will close it (or will just implement its closure);

However, it is still a bad idea to construct resources in the decoration of chaining them into wrapper constructors, due to several reasons I brought up.

---

P. S.

In any case, disregarding of how the resources are implemented or how you chain them or etc. everything, including JLS, Core Java book, OCP Java SE 8 book, and points brought here, indicate that it's always best to declare your resources separately.

---

Answer 2

After some searching I was able to find this article: https://dzone.com/articles/carefully-specify-multiple-resources-in-single-try

By definition JLS 14.20.3 a ResourceList consists of Resources separated by ;. Based on that we can conclude that a nested initialization like AutoClosable res = new Impl2(new Impl1()) is a single resource. Because of that rules defined for try-with-resources with multiple resources won't apply here, important ones being:

Resources are initialized in left-to-right order. If a resource fails to initialize (that is, its initializer expression throws an exception), then all resources initialized so far by the try-with-resources statement are closed. If all resources initialize successfully, the try block executes as normal and then all non-null resources of the try-with-resources statement are closed.

Resources are closed in the reverse order from that in which they were initialized. A resource is closed only if it initialized to a non-null value. An exception from the closing of one resource does not prevent the closing of other resources. Such an exception is suppressed if an exception was thrown previously by an initializer, the try block, or the closing of a resource.

What is more, Implt1#close() won't be called unless it is explicitly called inside of Impl2#close()

In short, it is better to declare multiple resources in separate statements separated with ;, like so:

try(Impl1 impl1 = new Impl1(); 
    Impl2 impl2 = new Impl2(impl1))

Answer 3

If the exception happens in the try part, there is no resource leakage (assuming Impl1 is written well). Notice that a raised exception in Impl1() will not reach Impl2 as the constructor argument is evaluated before calling it.

try (AutoClosable res = new Impl2(new Impl1())) {

So it is fine to nest such wrapping constructors; better style if the code does not become to long.

One remark: FileWriter and FileReader are old utility classes using the platform encoding, which will differ per application installation.

Path path = Paths.get(fileName);
try (BufferedWriter writer =
        Files.newBufferedWriter​(path, StandardCharsets.UTF8)) {