advice on nested Java try/finally code sandwiches

I would like some advice on a technique I bumped onto. It can be easily understood by looking at the code snippets, but I document it somewhat more in the following paragraphs.

Using the "Code Sandwich" idiom is commonplace to deal with resource management. Used to C++'s RAII idiom, I switched to Java and found my exception-safe resource management resulting in deeply nested code, in which I have a really hard time getting grip on the regular control flow.

Apparently (java data access: is this good style of java data access code, or is it too much try finally?, Java io ugly try-finally block and many more) I'm not alone.

I tried different solutions to cope with this:

1. maintain the program state explicitly: resource1aquired, fileopened..., and cleanup conditionally: if (resource1acquired) resource1.cleanup()... But I shun duplicating the program state in explicit variables - the runtime knows the state, and I don't want to care for it.

2. wrap every nested block in functions - results in even harder to follow control flow, and makes for really awkward function names: runResource1Acquired( r1 ), runFileOpened( r1, file ), ...

And finally I arrived at an idiom also (conceptually) backed by some research paper on code sandwiches:

Instead of this:

// (pseudocode)
try {
   connection = DBusConnection.SessionBus(); // may throw, needs cleanup
   try {
        exported = false;
        connection.export("/MyObject", myObject ); // may throw, needs cleanup
        exported = true;
            //... more try{}finally{} nested blocks
    } finally {
        if( exported ) connection.unExport( "/MyObject" );
    }   
} finally {
   if (connection != null ) connection.disconnect();
}

Using a helper construction, you may arrive at a more linear construct, in which compensation code is right next to the originator.

class Compensation { 
    public void compensate(){};
}
compensations = new Stack<Compensation>();

And the nested code becomes linear:

try {
    connection = DBusConnection.SessionBus(); // may throw, needs cleanup
    compensations.push( new Compensation(){ public void compensate() {
        connection.disconnect();
    });

    connection.export("/MyObject", myObject ); // may throw, needs cleanup
    compensations.push( new Compensation(){ public void compensate() {
        connection.unExport( "/MyObject" );
    });   

    // unfolded try{}finally{} code

} finally {
    while( !compensations.empty() )
        compensations.pop().compensate();
}

I was delighted: no matter how many exceptional p开发者_运维技巧aths, the control flow stays linear, and the cleanup code is visually next to the originating code. On top of that, it doesn't need an artificially restricted closeQuietly method, which makes it more flexible (i.e. not only Closeable objects, but also Disconnectable, Rollbackable and whatever others).

But...

I found no mention of this technique elsewhere. So here's the question:

Is this technique valid? What bugs do you see in it?

Thanks a lot.

I like the approach, but see a couple of limitations.

The first is that in the original, a throw in an early finally block doesn't affect later blocks. In your demonstration, a throw in the unexport action will stop the disconnect compensation happening.

The second is that it the language is complicated by the ugliness of Java's anonymous classes, including the need to introduce a heap of 'final' variables so they can be seen by the compensators. This is not your fault, but I wonder if the cure is worse than the disease.

But overall, I like the approach, and it is quite cute.

The way I see it, what you want are transactions. You compensations are transactions, implemented a bit differently. I assume that you are not working with JPA resources or any other resources that support transactions and rollbacks, because then it would be rather easy to simply use JTA (Java Transaction API). Also, I assume that your resources are not developed by you, because again, you could just have them implement the correct interfaces from JTA and use transactions with them.

So, I like your approach, but what I would do is hide the complexity of popping and compensating from the client. Also, you can pass transactions around transparently then.

Thus (watch out, ugly code ahead):

public class Transaction {
   private Stack<Compensation> compensations = new Stack<Compensation>();

   public Transaction addCompensation(Compensation compensation) {
      this.compensations.add(compensation);
   }

   public void rollback() {
      while(!compensations.empty())
         compensations.pop().compensate();
   }
}

A destructor like device for Java, that will be invoked at the end of the lexical scope, is an interesting topic; it's best addressed at language level, however the language czars don't find it very compelling.

Specifying destruction action immediately after construction action has been discussed (nothing new under the sun). An example is http://projectlombok.org/features/Cleanup.html

Another example, from a private discussion:

{
   FileReader reader = new FileReader(source);
   finally: reader.close(); // any statement

   reader.read();
}

This works by transforming

{
   A
   finally:
      F
   B
}

into

{
   A
   try
   {
      B
   }
   finally
   {
      F
   }
}

If Java 8 adds closure, we could implement this feature succinctly in closure:

auto_scope
#{
    A;
    on_exit #{ F; }
    B;
}

However, with closure, most resource library would provider their own auto cleanup device, clients don't really need to handle it by themselves

File.open(fileName) #{

    read...

}; // auto close

It is nice.

No major complaints, minor things off the top of my head:

• a little performance burden
• you need to make some things final for the Compensation to see them. Maybe this prevents some use-cases
• you should catch exceptions during compensate and continue running Compensations no matter what
• (bit far-fetched) you could accidentally empty out the Compensation queue at run-time due to a programming error. OTOH programming errors can happen anyway. And with your Compensation queue, you get "conditional finally blocks".
• don't push this too far. Within a single method it seems okay (but then you probably don't need too many try/finally blocks anyway), but don't pass Compensation queues up and down the call stack.
• it delays compensation to the "outermost finally" which could be a problem for things that need to be cleaned up early
• it only makes sense when you need the try block only for the finally block. If you have a catch block anyway, you might just add the finally right there.

Do you know you really need this complexity. What happens when you try to unExport something which is not exported?

// one try finally to rule them all.
try {
   connection = DBusConnection.SessionBus(); // may throw, needs cleanup
   connection.export("/MyObject", myObject ); // may throw, needs cleanup
   // more things which could throw an exception

} finally {
   // unwind more things which could have thrown an exception

   try { connection.unExport( "/MyObject" ); } catch(Exception ignored) { }
   if (connection != null ) connection.disconnect();
}

Using helper methods you could do

unExport(connection, "/MyObject");
disconnect(connection);

I would have thought disconnecting would mean you don't need to unExport the resources the connection is using.

You should look into try-with-resources, first introduced in Java 7. This should reduce the necessary nesting.