views:

137

answers:

4

In my application, I'm executing a transaction which can be in three different states at the end of the execution:

  • Success
  • Failure
  • Pending

Depending on the state, the application client will want to perform different actions. In the case of success, they will want to retrieve the transaction result (a Widget). In the case of failure, they will want to receive the error reason. In the case that the transaction is pending, they will want to schedule a time to retry. Currently I return an object with the following methods:

public interface Result() {
     State getState();
     Widget getWidget(); // Success
     Reason getFailureReason(); // Failure
     Callable<Result> getTask(); // Pending
}

The idea is that the client checks the state of the result object, and invokes the appropriate method depending on it's value, e.g.

if (result.getState() == State.PENDING) {
    result.getTask();
}

I was thinking that it might be preferable to use a callback instead, e.g.

public interface TransactionCallback() {
    void onFailure(Reason reason);
    void onSuccess(Widget widget);
    Delay onPending(Delay previous);
}

Where Delay is a class representing a TimeUnit and period, allowing the application to reschedule the transaction execution. Another alternative is to throw an exception wrapping the failure reason (as it should only fail in exceptional conditions), and keep the onSuccess and onPending methods.

So my question to SO is this: is the use of a callback an appropriate pattern for this particular problem, or can anyone suggest something more appropriate?

+3  A: 

I prefer callbacks since if you use it in several places it will result in less code and be more readable. With call back your if statement to determine what is going to happen (which method to call) will be in the service doing the transaction and all the code that uses the service will just look cleaner.

willcodejavaforfood
This was my reasoning too.
David Grant
Agreed. I find the more void methods and callbacks I use, the cleaner my code ends up being overall. Separating making the request, handling the result, and handling errors into separate methods (if not separate objects) seems to result in cleaner, more maintainable code.
kyoryu
+2  A: 

The callback arguments should include the object sourcing the call. Sounds fine for me, but using a callback could involve some synchronization issues - you can't be sure in which state you will receive the callback. Not impossible, but may need consideration.

Of course with polling the result, you would need synchronization on the other side. But that sounds simpler to synchronize.

ron
The callback is not asynchronous, so I'm not sure your concerns are valid *in this case*.
David Grant
But it could be asynchronous, if you run the workload in a separate thread. One nice thing about void methods and callbacks is that you can dispatch the work to a separate thread at any point. At any rate, even with a synchronous return, you can't be sure of the state of your object when you get the return value unless you lock the object anyway. I'd say it leads to better design in either case.
kyoryu
@David Ok. However the object may be included as an argument for the interface nevertheless, or you would have to set up some state in the receiver object for it to know where the results come from. Adding more state should be avoided if possible.
ron
@ron, the end user will not have visibility of the transaction, so I'm not sure if passing back that object will help the end user much. I have also been contemplating the user of a handback object which the end user could use to identify a particular operation.
David Grant
@ron, @kyoryu: might be time for me to dive back into Concurrency in Practice. ;)
David Grant
A: 

Callback solution is much extendable, while you may at one point want to change the behavior to an asynchronous call.

The drawback is your code will be less readable, especially to those beginner programmer. If that's not a problem for you then sure, go on...

nanda
+2  A: 

I don't think this is a good use of the callback pattern.

A callback is appropriate if the callee (e.g. the transaction in your case) needs to continue doing things after a callback method returns. But if the next thing that the callee always does is return to the caller, then the callback doesn't add any value. It is just makes the code structure more complex and less readable. IMO, it would be better to return a result object or (in the case of an exceptional failure) throw an exception.

EDIT - re the OP's comment.

I can see the value of using a callback method to ask the caller if the transaction should continue, though I probably would have used a simple timeout parameter. However, using callback methods to return results is (IMO) still wrong.

Stephen C
But this isn't always the case: if the transaction is in a *pending* state, the transaction will keep polling until the transaction succeeds or fails. The return from the onPending() method is required to know how long to wait until the next poll.
David Grant
I like the idea of eliminating the pending situation altogether with a timeout (although it is likely the time between requests would not necessarily be equal), but I'm not _quite_ convinced by your reluctance to use the callback. Do you have any references to back up your opinion?
David Grant
I agree with Stephen. Callbacks in this case only make the code harder to follow. I understand the desire to make the code extensible, but by sacrificing readability, you sacrifice maintainability. I think it's more likely that another programmer will not understand the code and make a mess than you some day taking advantage of the extensibility added by the callbacks.
Carlos
@David - my reference is common sense. Just compare the amount / complexity of code of the two ways of implementing this and you will see the difference.
Stephen C