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)){ ... }

Degeneration

I would say it strongly depends on the documented failures of the outer constructors. If they say they might fail, then sure, do it in separate resources. I don't think the specific example of BufferedWriter will fail, unless, say, the passed writer is null. – Scop

I think that doing it as a matter of course has the potential to cause other weird failures, because you'd end up closing the inner resources multiple times, if the outer resources' close() methods also close the inner resources. Again, this depends on the specific resources, since some (few? many? most?) will handle close idempotently. – Scop

Also there are some cases where the outer AutoClosable closes the inner AutoClosable in it's close method. – Yulandayule

I don't think that multiple close() may fail, as 1. each close is is done within a try-catch 2. no exception may be thrown from an implicit finally . However when you have such nested resources declaration, in this case we may declare a single resource but in other cases we'll have to declare multiple ones. I find it boring that we have to check if each constructor may fail. – Degeneration

@AndyTurner idempotent closing is mandated: “If the stream is already closed then invoking this method has no effect.” – Futhark

Answer 1

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))

Sabayon

With regard to the last sentence you can use: try(Impl1 impl1 = new Impl1(); Impl2 impl2 = new Impl2(impl1)) – Adulteration

@Adulteration Yes, that's exactly what I've meant - added example for clarity. – Sabayon

Answer 2

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)) {

Deon

What if exception is raised inside of Impl2 constructor? Impl1 is already created but never closed, right? – Sabayon

@Sabayon you are principally right; therefore I was grateful for your answer, but such wrapper classes typically should take care of these cases; an internal try-finally for instance. Having just said that new BufferedWriter(w, -1) (second constructor) will violate my claim. The shown code however is fine. – Deon

By the way there is many advise on exceptions in constructors. Also Buffered'Writer could have been written safe. – Deon

My question was not meant to target FileWriter and BufferedWriter. I think that, writting such a resource declaration, we shouldn't have to check what the constructors does, if there are hidden unchecked exceptions aso... I personnaly prefer to always declare 2 distinct resources to not have to do the check ; I was wondering if there was any problem with it and whether it should be a good practice. – Degeneration

Answer 3

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.

---

Typical

BufferedWriter#close() calls close() on the Writer passed in the constuctor, in this case both will be closed (unless something fails). – Sabayon