Menu
▾
▴
[Cdk-devel] [ cdk-Feature Requests-3437936 ] Readers and Writers should implement Closeable
|
From: SourceForge.net <no...@so...> - 2012-07-02 04:26:20
|
Feature Requests item #3437936, was opened at 2011-11-14 13:52 Message generated for change (Comment added) made by egonw You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=370024&aid=3437936&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: cdk.io Group: None >Status: Closed Priority: 5 Private: No Submitted By: Jonty (jontyl) Assigned to: Egon Willighagen (egonw) Summary: Readers and Writers should implement Closeable Initial Comment: 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 ---------------------------------------------------------------------- >Comment By: Egon Willighagen (egonw) Date: 2012-07-01 21:26 Message: What patch? Oh! That patch! (oops :) Patch has been applied since 14 Nov 2011. ---------------------------------------------------------------------- Comment By: Jonty (jontyl) Date: 2012-07-01 15:45 Message: Egon, Doesn't Rajarshi's patch referenced below work OK? If not then I can regenerate one, but it will be almost identical. Jonty ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2012-07-01 07:22 Message: Jonty, can you convert this into a git patch for master, please? ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-11-15 04:23 Message: 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 :) ---------------------------------------------------------------------- Comment By: Jonty (jontyl) Date: 2011-11-15 04:21 Message: 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. ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-11-14 22:20 Message: Rajarshi, your patch is for Closable, not AutoClosable... the latter is a new interface: http://download.oracle.com/javase/7/docs/api/java/lang/AutoCloseable.html ---------------------------------------------------------------------- Comment By: Rajarshi Guha (rajarshi) Date: 2011-11-14 17:43 Message: Patch supplied at https://sourceforge.net/tracker/?func=detail&aid=3437982&group_id=20024&atid=320024 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=370024&aid=3437936&group_id=20024 |