Discussion: New TransactionUtil.java Methods

[Adrian Crum-3]

Setting up transactions properly can be complicated. I've seen cases
where it was done wrong. Even when it's done right, it adds a lot of
client code that gets repeated every time a transaction is used. I have
an idea to make transaction handling easier and more reliable:

1. Create an interface:

public interface TransactionTask {
   void run() throws GenericEntityException;
}

2. Add a method to TransactionUtil.java:

public static void runTransactionTask(TransactionTask task, boolean
suspendCurrent) throws GenericEntityException, GenericTransactionException {
...
}

The TransactionUtil.runTransactionTask method will contain all of the
correct begin, suspend, commit, resume, rollback, try, catch, finally
logic. All the client code needs to do is put the transaction-protected
code in a TransactionTask instance run() method and call the
TransactionUtil.runTransactionTask method. Bottom line - less code,
better results.

Example:

TransactionTask task = new TransactionTask() {
     public void run() throws GenericEntityException {
         GenericValue target = delegator...;
         target.set("someField", "someValue");
         target.store();
     }
};
TransactionUtil.runTransactionTask(task, true);


What do you think?

-Adrian

[Bilgin Ibryam-2]

I like it, seems less error prone and I don't see any bad consequences using
it.

Bilgin

On Mon, Apr 25, 2011 at 5:13 PM, Adrian Crum <
[hidden email]> wrote:

> Setting up transactions properly can be complicated. I've seen cases where
> it was done wrong. Even when it's done right, it adds a lot of client code
> that gets repeated every time a transaction is used. I have an idea to make
> transaction handling easier and more reliable:
>
> 1. Create an interface:
>
> public interface TransactionTask {
>  void run() throws GenericEntityException;
> }
>
> 2. Add a method to TransactionUtil.java:
>
> public static void runTransactionTask(TransactionTask task, boolean
> suspendCurrent) throws GenericEntityException, GenericTransactionException {
> ...
> }
>
> The TransactionUtil.runTransactionTask method will contain all of the
> correct begin, suspend, commit, resume, rollback, try, catch, finally logic.
> All the client code needs to do is put the transaction-protected code in a
> TransactionTask instance run() method and call the
> TransactionUtil.runTransactionTask method. Bottom line - less code, better
> results.
>
> Example:
>
> TransactionTask task = new TransactionTask() {
>    public void run() throws GenericEntityException {
>        GenericValue target = delegator...;
>        target.set("someField", "someValue");
>        target.store();
>    }
> };
> TransactionUtil.runTransactionTask(task, true);
>
>
> What do you think?
>
> -Adrian
>
>

[Paul Foxworthy]

Hi Adrian,

The key point of difference is catches. The rest, as you say, is boilerplate stuff that can be encapsulated.

Client code could do whatever it wanted in catches, and if I'm reading you right you're proposing encapsulating a standardized catch. That might not be what the client wants under all circumstances.

Possible solutions are:

- No catch within runTransactionTask
- Standard catch within runTransactionTask, and rethrow the exception. I assume the catch would log the error.
- Add an overloaded variant of runTransactionTask with a boolean parameter to say whether or not the standard catch behaviour is wanted
- I did contemplate a system-wide property that can be configured to turn the standard catch on or off by default, but I think that would be too drastic, and client code should have the option case-by-case. I might be wrong.
- Don't use runTransactionTask if the standard catch is not wanted

These are not all mutually exclusive!

Do you have standard catch behaviour in mind?

Adrian Crum-3 wrote

Setting up transactions properly can be complicated. I've seen cases
where it was done wrong. Even when it's done right, it adds a lot of
client code that gets repeated every time a transaction is used. I have
an idea to make transaction handling easier and more reliable:

1. Create an interface:

public interface TransactionTask {
   void run() throws GenericEntityException;
}

2. Add a method to TransactionUtil.java:

public static void runTransactionTask(TransactionTask task, boolean
suspendCurrent) throws GenericEntityException, GenericTransactionException {
...
}

The TransactionUtil.runTransactionTask method will contain all of the
correct begin, suspend, commit, resume, rollback, try, catch, finally
logic. All the client code needs to do is put the transaction-protected
code in a TransactionTask instance run() method and call the
TransactionUtil.runTransactionTask method. Bottom line - less code,
better results.

Example:

TransactionTask task = new TransactionTask() {
     public void run() throws GenericEntityException {
         GenericValue target = delegator...;
         target.set("someField", "someValue");
         target.store();
     }
};
TransactionUtil.runTransactionTask(task, true);


What do you think?

-Adrian

[Adrian Crum-3]

GenericEntityException is caught, the transaction is rolled back, and the exception is re-thrown.

-Adrian


On 4/28/2011 7:24 PM, Paul Foxworthy wrote:

> Hi Adrian,
>
> The key point of difference is catches. The rest, as you say, is boilerplate
> stuff that can be encapsulated.
>
> Client code could do whatever it wanted in catches, and if I'm reading you
> right you're proposing encapsulating a standardized catch. That might not be
> what the client wants under all circumstances.
>
> Possible solutions are:
>
> - No catch within runTransactionTask
> - Standard catch within runTransactionTask, and rethrow the exception. I
> assume the catch would log the error.
> - Add an overloaded variant of runTransactionTask with a boolean parameter
> to say whether or not the standard catch behaviour is wanted
> - I did contemplate a system-wide property that can be configured to turn
> the standard catch on or off by default, but I think that would be too
> drastic, and client code should have the option case-by-case. I might be
> wrong.
> - Don't use runTransactionTask if the standard catch is not wanted
>
> These are not all mutually exclusive!
>
> Do you have standard catch behaviour in mind?
>
>
> Adrian Crum-3 wrote:
>> Setting up transactions properly can be complicated. I've seen cases
>> where it was done wrong. Even when it's done right, it adds a lot of
>> client code that gets repeated every time a transaction is used. I have
>> an idea to make transaction handling easier and more reliable:
>>
>> 1. Create an interface:
>>
>> public interface TransactionTask {
>>     void run() throws GenericEntityException;
>> }
>>
>> 2. Add a method to TransactionUtil.java:
>>
>> public static void runTransactionTask(TransactionTask task, boolean
>> suspendCurrent) throws GenericEntityException, GenericTransactionException
>> {
>> ...
>> }
>>
>> The TransactionUtil.runTransactionTask method will contain all of the
>> correct begin, suspend, commit, resume, rollback, try, catch, finally
>> logic. All the client code needs to do is put the transaction-protected
>> code in a TransactionTask instance run() method and call the
>> TransactionUtil.runTransactionTask method. Bottom line - less code,
>> better results.
>>
>> Example:
>>
>> TransactionTask task = new TransactionTask() {
>>       public void run() throws GenericEntityException {
>>           GenericValue target = delegator...;
>>           target.set("someField", "someValue");
>>           target.store();
>>       }
>> };
>> TransactionUtil.runTransactionTask(task, true);
>>
>>
>> What do you think?
>>
>> -Adrian
>>
>
> --
> View this message in context: http://ofbiz.135035.n4.nabble.com/Discussion-New-TransactionUtil-java-Methods-tp3473477p3482728.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

[David E. Jones-2]

You could just add methods to your interface to match up with other parts of the typical try/catch/finally blocks, such as:

void handleError(Throwable t);
void doFinally();

-David


On Apr 28, 2011, at 8:03 PM, Adrian Crum wrote:

> GenericEntityException is caught, the transaction is rolled back, and the exception is re-thrown.
>
> -Adrian
>
>
> On 4/28/2011 7:24 PM, Paul Foxworthy wrote:
>> Hi Adrian,
>>
>> The key point of difference is catches. The rest, as you say, is boilerplate
>> stuff that can be encapsulated.
>>
>> Client code could do whatever it wanted in catches, and if I'm reading you
>> right you're proposing encapsulating a standardized catch. That might not be
>> what the client wants under all circumstances.
>>
>> Possible solutions are:
>>
>> - No catch within runTransactionTask
>> - Standard catch within runTransactionTask, and rethrow the exception. I
>> assume the catch would log the error.
>> - Add an overloaded variant of runTransactionTask with a boolean parameter
>> to say whether or not the standard catch behaviour is wanted
>> - I did contemplate a system-wide property that can be configured to turn
>> the standard catch on or off by default, but I think that would be too
>> drastic, and client code should have the option case-by-case. I might be
>> wrong.
>> - Don't use runTransactionTask if the standard catch is not wanted
>>
>> These are not all mutually exclusive!
>>
>> Do you have standard catch behaviour in mind?
>>
>>
>> Adrian Crum-3 wrote:
>>> Setting up transactions properly can be complicated. I've seen cases
>>> where it was done wrong. Even when it's done right, it adds a lot of
>>> client code that gets repeated every time a transaction is used. I have
>>> an idea to make transaction handling easier and more reliable:
>>>
>>> 1. Create an interface:
>>>
>>> public interface TransactionTask {
>>>    void run() throws GenericEntityException;
>>> }
>>>
>>> 2. Add a method to TransactionUtil.java:
>>>
>>> public static void runTransactionTask(TransactionTask task, boolean
>>> suspendCurrent) throws GenericEntityException, GenericTransactionException
>>> {
>>> ...
>>> }
>>>
>>> The TransactionUtil.runTransactionTask method will contain all of the
>>> correct begin, suspend, commit, resume, rollback, try, catch, finally
>>> logic. All the client code needs to do is put the transaction-protected
>>> code in a TransactionTask instance run() method and call the
>>> TransactionUtil.runTransactionTask method. Bottom line - less code,
>>> better results.
>>>
>>> Example:
>>>
>>> TransactionTask task = new TransactionTask() {
>>>      public void run() throws GenericEntityException {
>>>          GenericValue target = delegator...;
>>>          target.set("someField", "someValue");
>>>          target.store();
>>>      }
>>> };
>>> TransactionUtil.runTransactionTask(task, true);
>>>
>>>
>>> What do you think?
>>>
>>> -Adrian
>>>
>>
>> --
>> View this message in context: http://ofbiz.135035.n4.nabble.com/Discussion-New-TransactionUtil-java-Methods-tp3473477p3482728.html
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

[Adrian Crum-3]

Good idea - thanks!

-Adrian

On 4/29/2011 9:52 AM, David E Jones wrote:

> You could just add methods to your interface to match up with other parts of the typical try/catch/finally blocks, such as:
>
> void handleError(Throwable t);
> void doFinally();
>
> -David
>
>
> On Apr 28, 2011, at 8:03 PM, Adrian Crum wrote:
>
>> GenericEntityException is caught, the transaction is rolled back, and the exception is re-thrown.
>>
>> -Adrian
>>
>>
>> On 4/28/2011 7:24 PM, Paul Foxworthy wrote:
>>> Hi Adrian,
>>>
>>> The key point of difference is catches. The rest, as you say, is boilerplate
>>> stuff that can be encapsulated.
>>>
>>> Client code could do whatever it wanted in catches, and if I'm reading you
>>> right you're proposing encapsulating a standardized catch. That might not be
>>> what the client wants under all circumstances.
>>>
>>> Possible solutions are:
>>>
>>> - No catch within runTransactionTask
>>> - Standard catch within runTransactionTask, and rethrow the exception. I
>>> assume the catch would log the error.
>>> - Add an overloaded variant of runTransactionTask with a boolean parameter
>>> to say whether or not the standard catch behaviour is wanted
>>> - I did contemplate a system-wide property that can be configured to turn
>>> the standard catch on or off by default, but I think that would be too
>>> drastic, and client code should have the option case-by-case. I might be
>>> wrong.
>>> - Don't use runTransactionTask if the standard catch is not wanted
>>>
>>> These are not all mutually exclusive!
>>>
>>> Do you have standard catch behaviour in mind?
>>>
>>>
>>> Adrian Crum-3 wrote:
>>>> Setting up transactions properly can be complicated. I've seen cases
>>>> where it was done wrong. Even when it's done right, it adds a lot of
>>>> client code that gets repeated every time a transaction is used. I have
>>>> an idea to make transaction handling easier and more reliable:
>>>>
>>>> 1. Create an interface:
>>>>
>>>> public interface TransactionTask {
>>>>     void run() throws GenericEntityException;
>>>> }
>>>>
>>>> 2. Add a method to TransactionUtil.java:
>>>>
>>>> public static void runTransactionTask(TransactionTask task, boolean
>>>> suspendCurrent) throws GenericEntityException, GenericTransactionException
>>>> {
>>>> ...
>>>> }
>>>>
>>>> The TransactionUtil.runTransactionTask method will contain all of the
>>>> correct begin, suspend, commit, resume, rollback, try, catch, finally
>>>> logic. All the client code needs to do is put the transaction-protected
>>>> code in a TransactionTask instance run() method and call the
>>>> TransactionUtil.runTransactionTask method. Bottom line - less code,
>>>> better results.
>>>>
>>>> Example:
>>>>
>>>> TransactionTask task = new TransactionTask() {
>>>>       public void run() throws GenericEntityException {
>>>>           GenericValue target = delegator...;
>>>>           target.set("someField", "someValue");
>>>>           target.store();
>>>>       }
>>>> };
>>>> TransactionUtil.runTransactionTask(task, true);
>>>>
>>>>
>>>> What do you think?
>>>>
>>>> -Adrian
>>>>
>>> --
>>> View this message in context: http://ofbiz.135035.n4.nabble.com/Discussion-New-TransactionUtil-java-Methods-tp3473477p3482728.html
>>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

[Jacques Le Roux]

I like the whole idea

Jacques

From: "Adrian Crum" <[hidden email]>

> Good idea - thanks!
>
> -Adrian
>
> On 4/29/2011 9:52 AM, David E Jones wrote:
>> You could just add methods to your interface to match up with other parts of the typical try/catch/finally blocks, such as:
>>
>> void handleError(Throwable t);
>> void doFinally();
>>
>> -David
>>
>>
>> On Apr 28, 2011, at 8:03 PM, Adrian Crum wrote:
>>
>>> GenericEntityException is caught, the transaction is rolled back, and the exception is re-thrown.
>>>
>>> -Adrian
>>>
>>>
>>> On 4/28/2011 7:24 PM, Paul Foxworthy wrote:
>>>> Hi Adrian,
>>>>
>>>> The key point of difference is catches. The rest, as you say, is boilerplate
>>>> stuff that can be encapsulated.
>>>>
>>>> Client code could do whatever it wanted in catches, and if I'm reading you
>>>> right you're proposing encapsulating a standardized catch. That might not be
>>>> what the client wants under all circumstances.
>>>>
>>>> Possible solutions are:
>>>>
>>>> - No catch within runTransactionTask
>>>> - Standard catch within runTransactionTask, and rethrow the exception. I
>>>> assume the catch would log the error.
>>>> - Add an overloaded variant of runTransactionTask with a boolean parameter
>>>> to say whether or not the standard catch behaviour is wanted
>>>> - I did contemplate a system-wide property that can be configured to turn
>>>> the standard catch on or off by default, but I think that would be too
>>>> drastic, and client code should have the option case-by-case. I might be
>>>> wrong.
>>>> - Don't use runTransactionTask if the standard catch is not wanted
>>>>
>>>> These are not all mutually exclusive!
>>>>
>>>> Do you have standard catch behaviour in mind?
>>>>
>>>>
>>>> Adrian Crum-3 wrote:
>>>>> Setting up transactions properly can be complicated. I've seen cases
>>>>> where it was done wrong. Even when it's done right, it adds a lot of
>>>>> client code that gets repeated every time a transaction is used. I have
>>>>> an idea to make transaction handling easier and more reliable:
>>>>>
>>>>> 1. Create an interface:
>>>>>
>>>>> public interface TransactionTask {
>>>>>     void run() throws GenericEntityException;
>>>>> }
>>>>>
>>>>> 2. Add a method to TransactionUtil.java:
>>>>>
>>>>> public static void runTransactionTask(TransactionTask task, boolean
>>>>> suspendCurrent) throws GenericEntityException, GenericTransactionException
>>>>> {
>>>>> ...
>>>>> }
>>>>>
>>>>> The TransactionUtil.runTransactionTask method will contain all of the
>>>>> correct begin, suspend, commit, resume, rollback, try, catch, finally
>>>>> logic. All the client code needs to do is put the transaction-protected
>>>>> code in a TransactionTask instance run() method and call the
>>>>> TransactionUtil.runTransactionTask method. Bottom line - less code,
>>>>> better results.
>>>>>
>>>>> Example:
>>>>>
>>>>> TransactionTask task = new TransactionTask() {
>>>>>       public void run() throws GenericEntityException {
>>>>>           GenericValue target = delegator...;
>>>>>           target.set("someField", "someValue");
>>>>>           target.store();
>>>>>       }
>>>>> };
>>>>> TransactionUtil.runTransactionTask(task, true);
>>>>>
>>>>>
>>>>> What do you think?
>>>>>
>>>>> -Adrian
>>>>>
>>>> --
>>>> View this message in context:
>>>> http://ofbiz.135035.n4.nabble.com/Discussion-New-TransactionUtil-java-Methods-tp3473477p3482728.html
>>>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

[Paul Foxworthy]

Hi Adrian,

Ah, the minimalist approach.

So, logging etc is the client's responsibility.

OK by me.

Cheers

Paul Foxworthy

Adrian Crum-3 wrote

GenericEntityException is caught, the transaction is rolled back, and the exception is re-thrown.

-Adrian


On 4/28/2011 7:24 PM, Paul Foxworthy wrote:
> Hi Adrian,
>
> The key point of difference is catches. The rest, as you say, is boilerplate
> stuff that can be encapsulated.
>
> Client code could do whatever it wanted in catches, and if I'm reading you
> right you're proposing encapsulating a standardized catch. That might not be
> what the client wants under all circumstances.
>
> Possible solutions are:
>
> - No catch within runTransactionTask
> - Standard catch within runTransactionTask, and rethrow the exception. I
> assume the catch would log the error.
> - Add an overloaded variant of runTransactionTask with a boolean parameter
> to say whether or not the standard catch behaviour is wanted
> - I did contemplate a system-wide property that can be configured to turn
> the standard catch on or off by default, but I think that would be too
> drastic, and client code should have the option case-by-case. I might be
> wrong.
> - Don't use runTransactionTask if the standard catch is not wanted
>
> These are not all mutually exclusive!
>
> Do you have standard catch behaviour in mind?
>
>
> Adrian Crum-3 wrote:
>> Setting up transactions properly can be complicated. I've seen cases
>> where it was done wrong. Even when it's done right, it adds a lot of
>> client code that gets repeated every time a transaction is used. I have
>> an idea to make transaction handling easier and more reliable:
>>
>> 1. Create an interface:
>>
>> public interface TransactionTask {
>>     void run() throws GenericEntityException;
>> }
>>
>> 2. Add a method to TransactionUtil.java:
>>
>> public static void runTransactionTask(TransactionTask task, boolean
>> suspendCurrent) throws GenericEntityException, GenericTransactionException
>> {
>> ...
>> }
>>
>> The TransactionUtil.runTransactionTask method will contain all of the
>> correct begin, suspend, commit, resume, rollback, try, catch, finally
>> logic. All the client code needs to do is put the transaction-protected
>> code in a TransactionTask instance run() method and call the
>> TransactionUtil.runTransactionTask method. Bottom line - less code,
>> better results.
>>
>> Example:
>>
>> TransactionTask task = new TransactionTask() {
>>       public void run() throws GenericEntityException {
>>           GenericValue target = delegator...;
>>           target.set("someField", "someValue");
>>           target.store();
>>       }
>> };
>> TransactionUtil.runTransactionTask(task, true);
>>
>>
>> What do you think?
>>
>> -Adrian
>>
>
> --
> View this message in context: http://ofbiz.135035.n4.nabble.com/Discussion-New-TransactionUtil-java-Methods-tp3473477p3482728.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

[Paul Foxworthy]

Hi David,

You could.

The advantage I see is one less layer of catch and rethrow. The disadvantage is the client needs to package catch and finally code into methods. More noise, a little more obscurity compared to simply writing the try-catch-finally around the call to runTransactionTask().

Of course, any client could write the try-catch-finally if they wish whether or not these two methods are there. Do the methods give enough benefit to justify complicating the API?

After contemplating more complicated possibilities, I think I'm coming around to Adrian's minimalist proposal.

Cheers

Paul Foxworthy

David E Jones-4 wrote

You could just add methods to your interface to match up with other parts of the typical try/catch/finally blocks, such as:

void handleError(Throwable t);
void doFinally();

-David


On Apr 28, 2011, at 8:03 PM, Adrian Crum wrote:

> GenericEntityException is caught, the transaction is rolled back, and the exception is re-thrown.
>
> -Adrian
>
>
> On 4/28/2011 7:24 PM, Paul Foxworthy wrote:
>> Hi Adrian,
>>
>> The key point of difference is catches. The rest, as you say, is boilerplate
>> stuff that can be encapsulated.
>>
>> Client code could do whatever it wanted in catches, and if I'm reading you
>> right you're proposing encapsulating a standardized catch. That might not be
>> what the client wants under all circumstances.
>>
>> Possible solutions are:
>>
>> - No catch within runTransactionTask
>> - Standard catch within runTransactionTask, and rethrow the exception. I
>> assume the catch would log the error.
>> - Add an overloaded variant of runTransactionTask with a boolean parameter
>> to say whether or not the standard catch behaviour is wanted
>> - I did contemplate a system-wide property that can be configured to turn
>> the standard catch on or off by default, but I think that would be too
>> drastic, and client code should have the option case-by-case. I might be
>> wrong.
>> - Don't use runTransactionTask if the standard catch is not wanted
>>
>> These are not all mutually exclusive!
>>
>> Do you have standard catch behaviour in mind?
>>
>>
>> Adrian Crum-3 wrote:
>>> Setting up transactions properly can be complicated. I've seen cases
>>> where it was done wrong. Even when it's done right, it adds a lot of
>>> client code that gets repeated every time a transaction is used. I have
>>> an idea to make transaction handling easier and more reliable:
>>>
>>> 1. Create an interface:
>>>
>>> public interface TransactionTask {
>>>    void run() throws GenericEntityException;
>>> }
>>>
>>> 2. Add a method to TransactionUtil.java:
>>>
>>> public static void runTransactionTask(TransactionTask task, boolean
>>> suspendCurrent) throws GenericEntityException, GenericTransactionException
>>> {
>>> ...
>>> }
>>>
>>> The TransactionUtil.runTransactionTask method will contain all of the
>>> correct begin, suspend, commit, resume, rollback, try, catch, finally
>>> logic. All the client code needs to do is put the transaction-protected
>>> code in a TransactionTask instance run() method and call the
>>> TransactionUtil.runTransactionTask method. Bottom line - less code,
>>> better results.
>>>
>>> Example:
>>>
>>> TransactionTask task = new TransactionTask() {
>>>      public void run() throws GenericEntityException {
>>>          GenericValue target = delegator...;
>>>          target.set("someField", "someValue");
>>>          target.store();
>>>      }
>>> };
>>> TransactionUtil.runTransactionTask(task, true);
>>>
>>>
>>> What do you think?
>>>
>>> -Adrian
>>>
>>
>> --
>> View this message in context: http://ofbiz.135035.n4.nabble.com/Discussion-New-TransactionUtil-java-Methods-tp3473477p3482728.html
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.