#176 Readers and Writers should implement Closeable

closed
cdk.io (23)
5
2014-08-15
2011-11-14
Jonty
No

Java 7 includes a feature to allow automatic closing of any object that implements the java.lang.AutoCloseable interface. The syntax is roughly:

try ( T resource = new T() ) { resource.use()... }

where T implements AutoCloseable. This is expanded by the compiler into code that contains a finally {} clause which calls resource.close(). This is superior to explicitly including a close() call because the generated code is designed to protect against all configurations where exceptions are thrown - in the resource.use() part and/or when calling close() itself. Note that all four combinations of exception throws are correctly dealt with. In particular, if use() throws an exception and close() then also throws an exception, the code is such that the close() exception will be added as a suppressed exception to the use() exception, and this latter will be rethrown.

It would be useful if this idiom could be used on CDK io classes. In fact this turns out to be very simple indeed to implement.

The AutoCloseable interface itself was only introduced in Java 7 so it should not be used directly to avoid backwards compatibility problems, but java.io.Closeable has existed since Java 5 and is appropriate for our needs. If the CDK is used on Java 6 or Java 5 runtimes then Closeable will not inherit AutoCloseable (which won't exist), but this will be undetected in code that doesn't use the idiom. Under a Java 7 runtime, AutoCloseable will exist and Closeable will extend it.

The desired behaviour can be achieved by having IChemObjectIO extend Closeable. IChemObjectIO already contains the method required by Closeable so there are no changes needed to anything that uses IChemObjectIO. It should be enough to make the source change and then recompile this single file in a JDK of 5 or later. All the reader and writer classes in io inherit indirectly from IChemObjectIO.

It is technically possible to write code that this change would break "if (ichemobjectio instanceof Closeable) DoSomethingStupid();" but I think it very highly unlikely that the change will even be noticed in any real world code not written explicitly to take advantage of it.

Jonty

Discussion

  • Jonty

    Jonty - 2011-11-14

    This is the changed file in com.openscience.cdk.io. You may accept this under whatever license you choose. Note this change has only been very lightly tested, but is unlikely to cause problems.

     
    Attachments
  • Jonty

    Jonty - 2011-11-14

    Short demonstration program that shows close() being correctly called. Must be compiled and run under Java 7.

     
    Attachments
  • Jonty

    Jonty - 2011-11-15

    Egon, Closeable is right because it exists pre Java 7 but in 7 it has been augmented to extend AutoClosable. Thus if you use Closeable it will achieve what we want on 7 but also be compatible with pre 7 versions.

     
  • Egon Willighagen

    Jonty, yeah, I realized that after I read your email... I left a comment, but that did not make it into the tracker, in which I indicated I was an idiot :)

     
  • Egon Willighagen

    Jonty, can you convert this into a git patch for master, please?

     
  • Jonty

    Jonty - 2012-07-01

    Egon,

    Doesn't Rajarshi's patch referenced below work OK? If not then I can regenerate one, but it will be almost identical.

    Jonty

     
  • Egon Willighagen

    What patch? Oh! That patch!

    (oops :)

    Patch has been applied since 14 Nov 2011.

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks