8

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?

rath
  • 856
  • 8
  • 20
  • 1
    Actually the if clause is fine! That's how it should be done. – synthomat Dec 24 '15 at 10:11
  • It doesn't :/ Both eclipse and netbeans complain. Note that with the if clause the variable is still uninitialized, I think that's what trips them up – rath Dec 24 '15 at 10:12
  • I don't get it. If it's not initialized, than the if clause won't be fulfilled. Did you try a "try-with-resources"? – synthomat Dec 24 '15 at 10:14
  • Not available in Java 6 I'm afraid. It could work with 7 and up though. – rath Dec 24 '15 at 10:19
  • 2
    @synthomat In Java, a `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. – amon Dec 24 '15 at 10:22
  • Not using "final" is a good solution, why not stick to it? Actually, your "in" variable should not be `final`, since you need the different states null/not null to make a decision to call close or not.. – Doc Brown Dec 24 '15 at 12:56
  • No, `in` must always close... except for the exceptional situation when an `Exception` has been thrown. If it can't close then it hasn't been opened, and if it hasn't been opened, the method has no reason to reach the close statement. – rath Dec 24 '15 at 13:06

2 Answers2

15

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't close() anything.
  • using the InputStream could fail, in which case you need a close().

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(); }
amon
  • 132,749
  • 27
  • 279
  • 375
  • 2
    Can get a bit verbose when constructing multiple resources, but that's Java for you. Thanks! – rath Dec 24 '15 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 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 Dec 24 '15 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 Dec 24 '15 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 Dec 28 '15 at 16:41
1

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.

Reinstate Monica
  • 1,851
  • 1
  • 11
  • 15