I like using final
variables whenever possible. Often those variables have to be closed afterwards. Note that I'm currently working on Java 6 so there is not closeable
interface, but the same applies to later Java versions as well. For example:
final InputStream in;
try{
in=new InputStream(...);
//...
}catch(...) {}
finally{
in.close(); //whoops!
}
At that point the IDE will point out that in
might be uninitialized. Wrapping the close
statement in an if like so
if(in!=null)
in.close();
doesn't work either, despite null
being the default value for uninitialized objects. My solution so far is to forget about declaring them final
in the first place, and initializing them as null
at declaration.
Another solution is to place the declaration and close
statement in the try
block:
try{
final InputStream in=new InputStream(...);
//...
in.close();
}catch(InputStreamRelatedException) {};
which would leave in
open (?) in case of non-InputStream
related exception.
What is the preferred method?
2 Answers 2
You are running into difficulties because you are trying to handle two different cases with the same try-catch:
- constructing the
InputStream
could fail, in which case you also can'tclose()
anything. - using the
InputStream
could fail, in which case you need aclose()
.
The solution is to separate these cases, and not instantiate the object in the same scope where it might be used:
final InputStream in;
try { in = new InputStream(); }
catch (...) { ... /* must throw or return, or assign to `in` */ }
try { doSomethingWith(in); }
catch (...) { ... }
finally { in.close(); }
-
2Can get a bit verbose when constructing multiple resources, but that's Java for you. Thanks!rath– rath2015年12月24日 10:31:40 +00:00Commented Dec 24, 2015 at 10:31
-
If you have many resources, you could create a type that closes a list of closeables, e.g.
class MultiCloseable implements Closeable { private List<Closeable> closeables = new ArrayList<>(); public void add(Closeable c) { c.add(c); } public void close() { for (Closeable c : closeables) { c.close(); } }
or something like that. Adding objects to that list could simplify cleanup.amon– amon2015年12月24日 11:41:33 +00:00Commented Dec 24, 2015 at 11:41 -
Good answer. I guess I would have nested the try's, however, because then you can join the declaration of
in
with its initialization... YMMV depending on actions of catching.Erik Eidt– Erik Eidt2015年12月24日 17:52:41 +00:00Commented Dec 24, 2015 at 17:52 -
@amon In that multi-closing code code you need to try/catch each close in case a close fails before other items have been closed.Reinstate Monica– Reinstate Monica2015年12月28日 16:41:46 +00:00Commented Dec 28, 2015 at 16:41
I agree with amon that there are two separate concerns but disagree with him/her over what those concerns are. By choosing a slightly different set of concerns you can write terser code. Also, amon's code block throws an IOException
because in.close()
's IOException
is not caught but my code does not so long as you catch the IOException
in the catch block.
IMO the two concerns are:
- an I/O error occurring at any point (in construction or use of the stream)
- closing the stream
With that in mind, we can write code like:
try {
final InputStream in = ... // assign in
try {
// use in
} finally {
in.close();
}
} catch (...) {} // if you catch IOException here, the block throws no IOException
Note that ideally you would put an additional try/catch around in.close()
to stop any exception thrown there from superseding the exception in the outer block. But that is horribly verbose.
As an aside, please do not use the anti-pattern of assigning null
to in
and checking for null
in the finally
block. It complicates the code for no good reason.
final
variable must be assigned to before you can read for it, and the compiler must be able to prove that a value has been assigned. No assignment will have happened if the constructor throws.