3

I have an Operation class like...

public sealed class Operation
{
  public void DoSomething1(ArgType1 arg) {...}
  public void DoSomething2(ArgType2 arg) {...}
  ...
  public Task<bool> Execute() {...}
}

The DoSomething methods package work to be done, storing the arg parameters and then the Execute() method will start a Task to accomplish this work together in an atomic fashion. The primary effect of the DoSomethings are side effects but some of them make sense to also return a value, my first instinct would be to return a Task as follows...

  public Task<ResultType3> DoSomething3(ArgType3 arg) {...}

but the catch is that that Task wouldn't be 'live' as most tasks are assumed to be. await-ing the result of this Task would be fruitless until Execute() is called to initiate the work, and thus I feel it would be confusing to the consumer. It is as if the return values of DoSomething3() and Execute() are non-independent tasks.

I could wrap the Task<> in a new type, called something like Result<>, and internally it would hold a Task<> and the Operation would hold onto its TaskCompletionSource<> and set the Result at the end of Execute() so that clients, after await-ing the Task returned from Execute, could observe the Result. 

public Result<T>
{ 
  internal Result(Task t) { _t = t; }
  public bool IsComplete { get { return _t.IsComplete; } }
  public T Result { get { return _t.Result; } }
  // Perhaps more methods delegating to the underlying Task
}

  public Result<ResultType4> DoSomething4(ArgType4 arg) {...}

The primary motivation of wrapping Task would be to communicate to the consumer that the result of DoSomething3() is not a live Task and to make it difficult / impossible to call...

var result = await op.DoSomething4(x);

as that would likely deadlock the code, since no one fired off the Operation yet. Note the similarity of this Result<> type to Nullable<> with different semantics.

Another approach would be for the method to return some opaque object that would be used as a key to retrieve the actual result from the Operation after Execute() has completed...

var token = op.DoSomething4(x);
...
var succeeded = await op.Execute();
if (! succeeded) return;
var result = op.RetrieveResult(token);

where Retrieve result would have a signature similar to...

public T RetrieveResult(Token<T> token) {...}

I suppose another option would be to add an additional argument that acts as a callback to be executed at the end of Execute() when the actual result is available...

public void DoSomething5(ArcType5 arg, Func<ResultType5,Task> callback) {...}

So as you can see I have several different options without a strong intuition about which is most appropriate. Unfortunately this is probably primarily just a matter of taste, but I would appreciate feedback on the different approaches.

i3arnon
  • 113,022
  • 33
  • 324
  • 344
John Jones
  • 41
  • 3
  • Is it absolutely necessary to return something from `DoSomethingN`? Could you not, for example, have a `Result` class that is returned by `Execute` (immediately) that has a `Task` member corresponding to the result of each `DoSomethingN`? Your consuming code could then look like: `op.EnqueueWork1(someDataForWork1); op.EnqueueWork1(someDataForWork2); var result = op.Execute(); var work2Result = await result.Work2Results;` – Asad Saeeduddin Feb 04 '15 at 05:54
  • Just to be clear, the `Result` I'm talking about isn't based on what you're describing in the question, it just has the same name. – Asad Saeeduddin Feb 04 '15 at 05:56
  • Why not let `Execute` receive an `Action` where `T` is your `ArgType` instead of separating these two methods which seem to be logically connected? – Yuval Itzchakov Feb 04 '15 at 06:51
  • I would just stick with `Task` returns and try to ensure that *naming* of the methods sets suitable expectations. Otherwise, it feels like you're basically re-implementing task with a different name and the user still has to go somewhere and learn your special rules about when it's appropriate to obtain the result values. – Damien_The_Unbeliever Feb 04 '15 at 07:59
  • Why are you returning a non-live `Task`? I did this on my utility methods when i first started using TPL and it always ended in confusion. – Gusdor Feb 04 '15 at 08:04
  • Thanks for your replies. The operation is a collection of atoms that will all be bundled and sent off for execution atomically (generally making database writes) The primary return result is really a key for a newly created row (we could call DoSomething, Create, or as Asad suggests EnqueueCreate) of course there are other actions that can be added to the Operation such as Modify and Delete as well as higher level ones and Create can be called multiple times in an operation with different args to generate multiple rows, the consumer would need to match the results with the actions. – John Jones Feb 04 '15 at 17:43
  • @JohnJones So, just to reiterate my question, is it absolutely necessary to have a return value from your `DoWhatever` methods? You could have them all be voids, and have `Execute` return an instance of a type that exposes a `Task` for each subresult corresponding to some queued work, and a `Task` for the overall completion. – Asad Saeeduddin Feb 07 '15 at 06:26
  • Based on feedback, I decided to have the methods return void and accept an optional lambda (Action) to capture the results, this way each piece of the operation remains isolated (it would be a little artificial to group all these basically independent results of varying types into a single bag). – John Jones Feb 10 '15 at 04:57

1 Answers1

2

I can't find a reason for you to have different methods that only set values (instead of properties for that matter) and a single method that runs everything.

But if you want to keep this design you can do something very similar to TPL Dataflow's blocks. Have a Completion Task property that completes only when Execute completes and have DoSomething3 be void. This lets the user understand that the entire operation can be awaited (including Execute) instead of just DoSomething3.

public sealed class Operation
{
    private TaskCompletionSource<bool> _tcs
    public Task Completion {get { return _tcs.Task;} }
    public void DoSomething3(ArgType2 arg) {...}
    ...
    public Task<bool> Execute() 
    {
        // ...
        _tcs.SetResult(false);
    }
}

Usage:

operation.DoSomething3(arg);
await operation.Completion;
i3arnon
  • 113,022
  • 33
  • 324
  • 344