Wednesday, May 30, 2012

try/with/resources refactoring that isn't


So I was looking at refactoring some code to use the new try/with/resources construct and found that perhaps it is not as simple as it first appears.

class Close implements AutoCloseable
   {
       @Override
       public void close() {
           System.out.println("Close");
       }
   }


   System.out.println("Normal version");


   Close close = new Close();
   try {
       throw new Error();
   }
   catch (Error e) {
       System.out.println("Catch");
   }
   finally {
       close.close();
   }

So the output for this is pretty easy to predict; but lets write it out just to be clear.

Normal version
Catch
Close

So this is pretty easy, lets throw some JDK 7 goodness on the code, this is an obvious refactoring right?

System.out.println("JDK 7 version");

   try (Close close2 = new Close()) {
       throw new Error();
   }
   catch (Error e) {
       System.out.println("Catch");
   }

So you can be pretty sure that given I am writing this up that the output is not going to be the same.


JDK 7 version
Close
Catch

So in this case the close is called before the catch, this might not be a problem but in some cases the catch block might need to make use of the resource before everything is shut down. If you want to perform a "refactoring" the code must behave the same before and after the changes. The following code is equivalent to the JDK 7 version without the try/with/resources

System.out.println("JDK 6 equivalent of JDK 7 version");

   Close close3 = new Close();
   try {
       try {
           throw new Error();
       }
       finally {
           close3.close();
       }
   }
   catch (Error e) {
       System.out.println("Catch");
   }


Just for completeness here is one possible refactoring of the original code which is functionally equivalent. Note quite as pretty I am afraid.

System.out.println("JDK 7 equivalent of original try/catch/finally");

   try (Close close4 = new Close()) {
       try {
         throw new Error();
       }
       catch (Error e) {
           System.out.println("Catch");
       }
   }

Now I ran this past my colleague Mark Warner and like me was surprised by the result. Mark pointed out that this nuance is noted in the java tutorial....

Note: A try-with-resources statement can have catch and finally blocks just like an ordinary try statement. In a try-with-resources statement, any catch or finally block is run after the resources declared have been closed.

...but it does feel like this rather important distinction is part of the small print that some people might at first glance assumed is related to the SQL example above it. (Yeah okay I didn't read the SQL example down to the end)

Again not something that will be a problem all the time; but it might be an important consideration in some quite important use cases.


No comments: