2

I have a simple class that does a synchronous thing,

public static class Synchronous
{
    public static void DoTheWholeThing()
    {
        AStuff aStuff;
        using (var a = new A())
        {
            aStuff = a.GetStuff();
        }

        BStuff bStuff;
        using (var b = new B())
        {
            bStuff = b.GetStuff();
        }

        var combination = CombineStuff(aStuff, bStuff);
    }

    private static Combination CombineStuff(AStuff aStuff, BStuff bStuff)
    {
        //// Magic Here
    }
}

Obviously, this code is not fully defined but it does illustrate my question.

Now, the classes A and B are both responsible for retrieving data from different remote sources. Consequently, the developers of A and B have implemented asynchronous entry points called GetStuffAsync which return Task<AStuff> and Task<BStuff> respectively.

I want to take maximum advantage of the asynchronous methods and call them concurrently so I can reduce the overall wait time of my code.

Here is what I've concocted, so far.

public static class Asynchronous
{
    public async static Task DoTheWholeThing(CancellationToken cancellationToken)
    {
        var getAStuffTask  = new Func<Task<AStuff>>(
                async () =>
                    {
                        using (var a = new A())
                        {
                            return await a.GetStuffAsync(cancellationToken);
                        }
                    })();

        var getBStuffTask  = new Func<Task<BStuff>>(
                async () =>
                    {
                        using (var b = new B())
                        {
                            return await b.GetStuffAsync(cancellationToken);
                        }
                    })();

        var combination = CombineStuff(
            await getAStuffTask,
            await getBStuffTask);
    }

    private Combination CombineStuff(AStuff aStuff, BStuff bStuff)
    {
        //// Magic Here
    }
}

Aside from this code looking curiously like the javascript module pattern, is this the correct approach. I don't think I should be using Task.Run as this code is clearly not CPU bound.

It seems a bit "clunky" that I need to instantiate typed delegates to do this. Is there a better way?

EDIT

following two good answers I'm in a quandary between named functions and continuations.

Jodrell
  • 34,946
  • 5
  • 87
  • 124
  • Concerning your edit - pick the solution lowest complexity. This may be the solution with more code. Failing that, pick the solution you find easier to parse. I'd actually stick with anonymous methods as (personally) I would struggle to name the methods, and the continuation solution assumes more knowledge. – Gusdor Sep 04 '15 at 07:08

3 Answers3

3

The code becomes radically simpler when you simply extract the anonymous methods out into named methods:

public async static Task DoTheWholeThing(CancellationToken cancellationToken)
{
    var getAStuffTask = GetAStuffAsync(cancellationToken);
    var getBStuffTask = GetBStuffAsync(cancellationToken);

    var combination = CombineStuff(
        await getAStuffTask,
        await getBStuffTask);
}

private static async Task<AStuff> GetAStuffAsync(CancellationToken cancellationToken)
{
    using (var a = new A())
    {
        return await a.GetStuffAsync(cancellationToken);
    }
}
private static async Task<BStuff> GetBStuffAsync(CancellationToken cancellationToken)
{
    using (var b = new B())
    {
        return await b.GetStuffAsync(cancellationToken);
    }
}

That said, if you really want to stick with the anonymous methods, you can create a helper method that will allow generic type inference and lambdas to implicitly figure out the type of the delegate:

public async static Task DoTheWholeThing(CancellationToken cancellationToken)
{
    var getAStuffTask = Start(async () =>
            {
                using (var a = new A())
                {
                    return await a.GetStuffAsync(cancellationToken);
                }
            });

    var getBStuffTask = Start(async () =>
            {
                using (var b = new B())
                {
                    return await b.GetStuffAsync(cancellationToken);
                }
            });

    var combination = CombineStuff(
        await getAStuffTask,
        await getBStuffTask);
}
public static Task<T> Start<T>(Func<Task<T>> asyncOperation)
{
    return asyncOperation();
}
Servy
  • 202,030
  • 26
  • 332
  • 449
  • This should be combined with `Task.WhenAll` to simplify the call to `CombineStuff`, for debugging purposes if nothing else. – Panagiotis Kanavos Sep 03 '15 at 15:09
  • It will be the best answer, if combined with `WhenAll` trick to await tasks in parallel as topic starter requested. – Uladzislaŭ Sep 03 '15 at 15:09
  • @PanagiotisKanavos That pretty much just comes down to personal preference and coding style. If the OP wants to structure his code this way, it's a perfectly acceptable option. If he, or you, want to use `WhenAll`, then that's fine to. – Servy Sep 03 '15 at 15:10
  • @Vladislav No, there is no need to use `WhenAll`. The operations are *already* running in parallel. This isn't even a functional change of the OP's code, it's simply a refactor of the code by extracting the anonymous methods out into named methods. It already functioned exactly as intended. – Servy Sep 03 '15 at 15:11
  • @Servy when trying to debug the statement it will be kind of awkward if stepping into is delayed by waiting for both tasks to complete. Eg putting a breakpoint on `CombineStuff` wouldn't actually stop after the data is returned – Panagiotis Kanavos Sep 03 '15 at 15:17
  • @PanagiotisKanavos Indeed. Like I said, it's a personal preference. You may prefer to add the call to `WhenAll`. Others prefer to not add it. In the same way that some people will write `Foo(Bar(), Baz())` and others may prefer to store the results of `Bar` and `Baz` in local variables instead. Each option has their merits and their problems. – Servy Sep 03 '15 at 15:20
  • On examination, I can't say that the anonymous approach is either more compact or more readable. I shall almost certainly accept this as the answer. I'm just wondering if there is something that can be achieved with the c# 6.0 expression bodied function sugar. http://stackoverflow.com/questions/28411335/expression-bodied-function-members-efficiency-and-performance-in-c-sharp-6-0 – Jodrell Sep 03 '15 at 15:27
  • @Jodrell No, as you have a statement, not an expression, as the body of the method. You can't use an expression bodied member for the same reason that you had to use a statement lambda over an expression lambda. And I agree that the named method approach is better; that's why its at the top. – Servy Sep 03 '15 at 15:29
3

Use TPL continuations to call Dispose as soon as the task is complete.

public async static Task DoTheWholeThing(CancellationToken cancellationToken)
{
    var a = new A();
    var b = new B();

    // start the tasks and store them for awaiting later
    var getAStuffTask = a.GetStuffAsync(cancellationToken);
    var getBStuffTask = b.GetStuffAsync(cancellationToken);

    // queue up continuations to dispose of the resource as soon as it is not needed
    getAStuffTask.ContinueWith(() => a.Dispose());
    getBStuffTask.ContinueWith(() => b.Dispose());

    // await as normal
    var combination = CombineStuff(
        await getAStuffTask,
        await getBStuffTask);
}

I am unsure if wrapping the whole method in an addition using block will accomplish anything but it may provide peace of mind.

Gusdor
  • 14,001
  • 2
  • 52
  • 64
  • potentially good, this seems pertitnent, http://stackoverflow.com/a/26004975/659190 – Jodrell Sep 03 '15 at 15:41
  • This question is, if `a.GetStuffAsync` throws an exception, will both continuations occur whatever the current state of each task? Obviously I'd like to be sure. – Jodrell Sep 03 '15 at 15:48
  • @Jodrell Yes, they will. I said as much in the answer you linked above. – Servy Sep 03 '15 at 15:48
  • @Jodrell a continuation will run regardless of the final state of the task if you omit ContinuationOptions. This solution is very similar to how using blocks interact with await – Gusdor Sep 03 '15 at 17:23
1

You don't need to wrap your async calls in delegates to get them to execute immediately. If you call the GetStuffAsync methods directly without awaiting them you will have the same result.

public static class Asynchronous
{
    public async static Task DoTheWholeThing(CancellationToken cancellationToken)
    {
        using (var a = new A())
        using (var b = new B()) {
            var taskA = a.GetStuffAsync(cancellationToken);
            var taskB = b.GetStuffAsync(cancellationToken);
            await Task.WhenAll(new [] { taskA, taskB });
            var combination = CombineStuff(taskA.Result, taskB.Result);
        }
    }

    private Combination CombineStuff(AStuff aStuff, BStuff bStuff)
    {
        //// Magic Here
    }
}

Note that this does keep the a and b objects alive during the call to CombineStuff as @Servy notes. If that is a problem the declaration of the Task objects can be moved outside of the using blocks as below:

public static class Asynchronous
{
    public async static Task DoTheWholeThing(CancellationToken cancellationToken)
    {
        Task taskA;
        Task taskB;
        using (var a = new A())
        using (var b = new B()) {
           taskA = a.GetStuffAsync(cancellationToken);
           taskB = b.GetStuffAsync(cancellationToken);
           await Task.WhenAll(new [] { taskA, taskB });
         }

         var combination = CombineStuff(taskA.Result, taskB.Result);
    }

    private Combination CombineStuff(AStuff aStuff, BStuff bStuff)
    {
        //// Magic Here
    }
}

Although this still holds onto a and b as long as both tasks are running, rather than disposing of each as they return.

shf301
  • 31,086
  • 2
  • 52
  • 86
  • 1
    Note that both of these are now holding onto the disposable resources for much longer. – Servy Sep 03 '15 at 14:55
  • After your edit it still holds on to the disposable resources for longer than it needs to (it holds onto each until both finish, rather than simply the one call that uses it) but it's certainly less bad as it doesn't wait for `CombineStuff` to finish as well. – Servy Sep 03 '15 at 15:13
  • @Servy Yes you are correct, I've updated my answer to make that clear. It would depend on how critical early Disposal is. – shf301 Sep 03 '15 at 15:17