Juan Erroa
Juan Erroa

Reputation: 45

Best way to call functions asynchronously

I trying get a string to set three label text asynchronously, Firstly I tried with this

private async void button1_Click(object sender, EventArgs e)
{
    label1.Text = await SetTextbox(3);
    label2.Text = await SetTextbox(2);
    label3.Text = await SetTextbox(1);
}

private async Task<string> SetTextbox(int delaySeconds)
{
    Thread.Sleep(delaySeconds * 1000);
    return "Done.";
}

As you can see I try to add some delay in every call to know what label text should be set first, but this code doesn't work, the GUI still freezing and the processes still being synchronous.

For now I make it work using Task.Run:

private async void button1_Click(object sender, EventArgs e)
{
     var proc1 = Task.Run(async () => { label1.Text = await SetTextbox(3); });
     var proc2 = Task.Run(async () => { label2.Text = await SetTextbox(2); });
     var proc3 = Task.Run(async () => { label3.Text = await SetTextbox(1); });

     await Task.WhenAll(proc1, proc2, proc3);
}

but something tells me that this is not right way to achieve this, Can you tell me what would be the best way to do this or this is good solution?

Upvotes: 0

Views: 511

Answers (3)

Theodor Zoulias
Theodor Zoulias

Reputation: 44026

The SetTextbox method is not a well behaved asynchronous method. The expected behavior of an asynchronous method is to return a Task immediately. Blocking the caller with Thread.Sleep breaks this expectation.

Now if we are handed with a badly behaving asynchronous method, Task.Run is our friend. Important: the only thing that should be wrapped inside the Task.Run is the blocking asynchronous method. Any UI related code should stay outside:

private async void Button1_Click(object sender, EventArgs e)
{
     var task1 = Task.Run(async () => await BadBlockingMethodAsync(1));
     var task2 = Task.Run(async () => await BadBlockingMethodAsync(2));
     var task3 = Task.Run(async () => await BadBlockingMethodAsync(3));

     await Task.WhenAll(task1, task2, task3);

     label1.Text = await task1;
     label2.Text = await task2;
     label3.Text = await task3;
}

As a side note, SetTextbox is an improper name for an asynchronous method. According to the guidelines the method should have an Async suffix (SetTextboxAsync).

Upvotes: 2

Rahul
Rahul

Reputation: 77934

Setting those 3 labels are not dependent operations right. In that case, you can run all the method parallel and get their result once all done like

private async Task button1_Click(object sender, EventArgs e)
{
     var t1 = SetTextbox(3);
     var t2 = SetTextbox(2);
     var t3 = SetTextbox(1);

     string[] data = await Task.WhenAll(new[] { t1, t2, t3 });

    label1.Text = data[0];
    label2.Text = data[1];
    label3.Text = data[2];

} 

Upvotes: 1

Bibipkins
Bibipkins

Reputation: 471

Asynchronous does not mean that it will execute in parallel - which is what you need if you want UI to stop freezing. And that is why Task.Run works. Have a look at this question
Your Task.Run solution should be ok - it is one of C# intended ways of running something in parallel

Upvotes: 0

Related Questions