use try-with-resources to close a Closeable

Question

I have a Map<Key, Closeable> and if a key is removed from the map I want to close the Closeable. Normally I have something like:

Closeable c = map.remove(key);
c.close();

My Eclipse warns me "Resource 'c' should be managed by try-with-resource", so is it better just to write the following?

try (Closeable c = map.remove(key)) {}

In my special implementation I have a subclass of Closeable, where close() does not throw an IOException, so no exception handling is needed.

It is good to use `try-with-resources` statement, so what is your question? — Yu Jiaao, Jun 30 '17 at 09:48
Can you give more details about the Closeable objects, why they are put in a map and why they need to be closed when they are removed from that map? — WilQu, Jun 30 '17 at 09:52
You could eliminate `c` by writing `map.remove(key).close()`. — Kevin Krumwiede, Jun 30 '17 at 10:15
@Jerry06 I know what it does, I just want to know the best practice. @YuJiaao As matoni stated in his answer "Empty try-with-resource looks weird.", so I asked. @KevinKrumwiede You are right, but this is not the question. @WilQu It is used in a ServerSocket process. The map saves the active client connections. On disconnect or timeout they are removed from this map and the listening `Thread` will be closed. — Madjosz, Jun 30 '17 at 11:05

score 5 · Accepted Answer · answered Jun 30 '17 at 14:04

The point of try-with-resources is that:

The opening of the Closeable resource is done in the try statement
The use of the resource is inside the try statement's block
close() is called for you automatically.

So your suggested code:

try(Closeable c = map.remove(key)) {}

... doesn't satisfy the point of try-with-resource, since you're not using the resource inside the block. Presumably your Closeable is already open before this statement.

I'm guessing that you have some code whereby a bunch of resources are opened, work is done, then they are all closed by working through the map.

This is OK, and sometimes unavoidable. But it's cleaner, where possible, to have open() and close() in the same method, with the close() in a finally block, so that you can see at a glance that every open() has a corresponding close() and you can be sure that the close() is always called.

MyCloseable c = MyCloseable.open(...);
try{
       // do stuff with c;
} finally {
     try {
         c.close();
     } catch (IOException e) {
         // ...
     }
}

Once you've achieved that, try-with-resources just makes things neater:

try(MyCloseable c = MyCloseable.open(...)) {
    // do stuff with c;
}

If your requirements mean you can't get open and close into the same methods, then just stick with an explicit close() and ignore the warning.

Theoretical the resources can be open forever since it is a Client-Server communication and the client can be logged in forever if there is no timeout. — Madjosz, Jul 03 '17 at 11:52

matoni · Answer 2 · 2017-06-30T12:04:44.637

I would just ignore this warning, If you are managing close operation on your own, then just call close(). Empty try-with-resource looks weird.

Consider to extend Map so close operation will be performed automatically on remove:

public class CloseableMap<K,V extends Closeable> extends HashMap<K,V> {

    @Override
    public R remove(K key) {
        V resource = super.remove(key);
        if (resource != null) {
            resource.close();
        }
        return resource;
    }
}

use try-with-resources to close a Closeable

2 Answers2