Best way to call functions asynchronously

Question

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?

I would also advice the use of `await Task.Delay(delaySeconds * 1000)` instead of the `Thread.Sleep`. The first one is made for use with `async` the latter is for when you're using threads. — , Apr 03 '20 at 22:18
What if you just change `Thread.Sleep(delaySeconds * 1000);` to `await Task.Delay(delaySeconds * 1000);` — Wyck, Apr 04 '20 at 01:31
Thread.Sleep() can be (and should be) used to simulate _synchronous_ work. Task.Delay() would be wrong in that case. — H H, Apr 05 '20 at 11:15

score 2 · Accepted Answer · answered Apr 03 '20 at 22:16

2

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).

answered Apr 03 '20 at 22:16

Theodor Zoulias

34,835
7
69
104

SetTextbox is not async at all. Async (behaviour) does not come from the `async` keyword but from `await`ing something that _is_ async. – H H Apr 05 '20 at 08:48
@Henk Holterman according to the official docs, asynchronous is a method that returns an awaitable type. The `SetTextbox` returns `Task`, which is awaitable, so the method is asynchronous. *"Asynchronous methods in TAP include the Async suffix after the operation name for methods that return awaitable types, such as Task, Task, ValueTask, and ValueTask."* ([citation](https://learn.microsoft.com/en-us/dotnet/standard/asynchronous-programming-patterns/task-based-asynchronous-pattern-tap#naming-parameters-and-return-types)). – Theodor Zoulias Apr 05 '20 at 08:53
It is awaitable, yes. But it is not really async. It is faking it, as per the compiler warning. And in the first sample everything runs on the main thread, synchronously and sequentially. – H H Apr 05 '20 at 08:59
@HenkHolterman I agree that the method fakes to be asynchronous based on its implementation, but having a name that ignores the official naming recommendations doesn't make things any better. – Theodor Zoulias Apr 05 '20 at 09:16
Still, the better solution would be to keep the name and change the signature. You can still run a `void M() {}` with Task.Run() and it would be more honest. And avoid some useless async/await overhead at the same time. – H H Apr 05 '20 at 11:06
@HenkHolterman the `SetTextbox` method is currently so obscure regarding its intentions, that I wouldn't dare suggesting architectural improvements. – Theodor Zoulias Apr 05 '20 at 11:23
1

Yes, when posting my answer I realized it should be called `GenerateString[Async](int)`, it is not setting a TextBox at all. And it shouldn't. – H H Apr 05 '20 at 11:40

Rahul · Answer 2 · 2020-04-03T22:03:21.753

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];

}

score 0 · Answer 3 · answered Apr 03 '20 at 21:55

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

Best way to call functions asynchronously

3 Answers3