Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I've been trying to learn how to use the TPL for quite some time now. Basically I'm trying to fire off a group of Tasks, wait for them to complete and then fire off another group of tasks, waiting for them to complete before firing off the tasks that update form elements on the UI Thread.

Here's what I got. I think I'm probably overcomplicating it however it does seem to work as I want it too. Please could I have some advice on how I could simplify this based on what I want to achieve?

private async Task PrepareData()
{
    Task[] FirstTasks = new Task[] { TaskOne(), TaskTwo() };    // Do First group of Tasks
    await Task.WhenAll(FirstTasks); // Wait for First group of Tasks

    Task[] SecondTasks = new Task[] { TaskThree(), TaskFour() }; // Do Second group of Tasks
    await Task.WhenAll(SecondTasks).ContinueWith(c => // Wait for Second group of Tasks 
        {
            GuiTaskOne();
            GuiTaskTwo();
        }, TaskScheduler.FromCurrentSynchronizationContext());  // Then update the form on the UI Thread
}

private async Task TaskOne()
{
    someVariable = await Task.Run(() => DoSomething());
}

private async Task TaskTwo()
{
    someVariable = await Task.Run(() => DoSomething());
}

private async Task TaskThree()
{
    someVariable = await Task.Run(() => DoSomething());
}

private async Task TaskFour()
{
    someVariable = await Task.Run(() => DoSomething());
}

private async Task GuiTaskOne()
{
    someControl.Text = await Task.Run(() => DoSomething());
}

private async Task GuiTaskTwo()
{
    someControl.Text = await Task.Run(() => DoSomething());
}
share|improve this question

closed as off-topic by RubberDuck, SuperBiasedMan, Morwenn, Alien Herb Nite, hjpotter92 Oct 8 '15 at 11:54

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

    
I presume DoSomething() is just a method that runs something like Thread.Sleep(1000);? – Mat's Mug Feb 24 '14 at 13:47
    
It could be. I'm using the above code in a real world scenario. The FirstTasks Task array does a couple of Active Directory lookups where the the SecondTasks Task Array uses the information gathered from AD to populate SQL databases. The GUI tasks report progress/information. Thanks – Robula Feb 24 '14 at 15:03
2  
I think you'd get a better review and more useful comments if you posted your actual code - at least the names for TaskThree and GuiTaskOne (and the others).. right now it looks like all tasks call the same DoSomething method. I know in reality it's probably not the case, but since reviewers can comment on any aspect of your code, it'd be nice if your code wasn't too narrowed down on the async/await mechanics. – Mat's Mug Feb 24 '14 at 15:14
    
Have a search on TPL Dataflow – Larry Feb 1 at 13:55
up vote 7 down vote accepted

You can simplify Gaz's code even more by using the fact that WhenAll() is a params method:

private async Task PrepareData()
{
    await Task.WhenAll(TaskOne(), TaskTwo());

    await Task.WhenAll(TaskThree(), TaskFour());

    GuiTaskOne();
    GuiTaskTwo();
}

Also, depending on what the Task methods actually do, there might be a better way than having them set a field, but that's hard to tell from the example code.

share|improve this answer

Since await is effectively a continuation anyway, you could simplify your function to:

private async Task PrepareData()
{
    Task[] FirstTasks = new Task[] { TaskOne(), TaskTwo() };    // Do First group of Tasks
    await Task.WhenAll(FirstTasks); // Wait for First group of Tasks

    Task[] SecondTasks = new Task[] { TaskThree(), TaskFour() }; // Do Second group of Tasks
    await Task.WhenAll(SecondTasks); // Wait for Second group of Tasks 

    GuiTaskOne();
    GuiTaskTwo();
}

I also suspect you meant to await the GUI tasks too.

share|improve this answer
await Task.WhenAll(Task.Delay(TimeSpan.FromSeconds(5)), Task.Delay(TimeSpan.FromSeconds(5)), Task.Delay(TimeSpan.FromSeconds(5)))
            .ContinueWith(_ => Task.WhenAll(Task.Delay(TimeSpan.FromSeconds(5)), Task.Delay(TimeSpan.FromSeconds(5)), Task.Delay(TimeSpan.FromSeconds(5))))
            .Unwrap();

Note that it is important to call Unwrap when your action returns Task<Task> rather than Task<TResult>.

The above statement should take 10 seconds. Without Unwrap, it would take 5 seconds because it will not 'await' for the second group to complete.

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.