What is wrong using Async and Await in this WPF example?

Question

I'm new to await async, I want to make sense of what I studied about the subject in this real scenario:

I have a simple code that reads bitcoin price which takes 1-2 seconds, I don't want to lock the UI using await async and still give a status if it is loading or done:

    private void button_Click(object sender, RoutedEventArgs e)
    {
        Task<int> bitcoinPriceTask = GetBitcoinPrice();
        lblStatus.Content = "Loading...";
    }

    protected async Task<int> GetBitcoinPrice()
    {
        IPriceRetrieve bitcoin = new BitcoinPrice();
        string price = bitcoin.GetStringPrice();
        txtResult.Text = price;
        lblStatus.Content = "Done";
        return 1;
    }

as requested, here is the implementation of BitcoinPrice class:

public class BitcoinPrice : IPriceRetrieve
{
    public BitcoinPrice()
    {
        Url = "https://www.google.com/search?q=bitcoin%20price";
    }

    public string Url { get; }


    public string GetStringPrice()
    {
        var html = RetrieveContent();
        html = MetadataUtil.GetFromTags(html, "1 Bitcoin = ", " US dollars");
        return html;
    }

    public float GetPrice()
    {
        throw new NotImplementedException();
    }

    public string RetrieveContent()
    {
        var request = WebRequest.Create(Url);
        var response = request.GetResponse();
        var dataStream = response.GetResponseStream();
        var reader = new StreamReader(dataStream);
        var responseFromServer = reader.ReadToEnd();
        return responseFromServer;
    }
}

Think about this, your question is "What is wrong using Async and ***Await*** in this WPF example?" please point to me the line that uses `await`. — Scott Chamberlain, Mar 18 '16 at 21:21
`WebRequest` is a little heavy weight, you can replace all of `RetrieveContent()` with `public async Task RetrieveContent() { using(var wc = new WebClient()) { return await wc.DownloadStringAsync(Url); } }` then edit GetStringPrice to start with `var html = await RetrieveContent();` — Scott Chamberlain, Mar 18 '16 at 21:39

Ehsan Sajjad · Accepted Answer · 2016-03-18T21:39:22.630

7

Your code right now has many issues, first of all you need your event handler to be async so that you can await on your method which is returning Task<int>, secondly you can set message Loading before calling method and await it so that it waits for that method to complete it's and when it will complete working returning result then set message to Done:

private async void button_Click(object sender, RoutedEventArgs e)
{
     lblStatus.Content = "Loading...";
     int bitcoinPriceTask = await GetBitcoinPrice();
     lblStatus.Content = "Done";

}

protected async Task<int> GetBitcoinPrice()
{
     IPriceRetrieve bitcoin = new BitcoinPrice();
     string price = await bitcoin.GetStringPrice();
     txtResult.Text = price;
     return 1;
}

or more better can be returning Task<string> and set the TextBox value there in event handler:

protected async Task<string> GetBitcoinPrice()
{
    IPriceRetrieve bitcoin = new BitcoinPrice();
    string price = await bitcoin.GetStringPrice();
    return price;
}

and in event handler:

private async void button_Click(object sender, RoutedEventArgs e)
{
     lblStatus.Content = "Loading...";
     string price = await GetBitcoinPrice();
     txtResult.Text = price;
     lblStatus.Content = "Done";

}

edited Mar 18 '16 at 21:39

answered Mar 18 '16 at 21:23

Ehsan Sajjad

61,834
16
105
160

In your "better version" you could also throw in a `.ConfigureAwait(false)` on `GetStringPrice()` because you no-longer touch the UI inside the `GetBitcoinPrice()` method. – Scott Chamberlain Mar 18 '16 at 21:29
thanks, not sure if I missed something but it still locks the ui – RollRoll Mar 18 '16 at 21:29
@ThePoet the thing you likely missed is `BitcoinPrice.GetStringPrice()` needs to also return a `Task`. The entire method chain should be async till you get to the network call you are making to find the price. (You also could wrap it in a `Task.Run(` but making the call chain async to the network call is the much better option) – Scott Chamberlain Mar 18 '16 at 21:30
@ThePoet what is the implemetation of ``GetBitcoinPrice()``? – Ehsan Sajjad Mar 18 '16 at 21:32
so if I wanted the methods of my classes to be able to use awaitAsync from who is consuming it, I always have to change the signature to Task ? – RollRoll Mar 18 '16 at 21:32
@ThePoet yes, till you get to code you did not write that returns a `Task` for example you likely are using `WebClient.DownloadString(` which returns a `string`, you would change your code to call `WebClient.DownloadStringAsync(` which returns a `Task` then you would await on that task in your function. – Scott Chamberlain Mar 18 '16 at 21:34
yes, you have to, see : http://stackoverflow.com/questions/16063520/how-do-you-create-an-async-method-in-c – Ehsan Sajjad Mar 18 '16 at 21:34
I added the BitcoinPrice code in my question, as requested – RollRoll Mar 18 '16 at 21:35
@ThePoet you have to make that method async as well – Ehsan Sajjad Mar 18 '16 at 21:36
thank you. Very stupid question(maybe): Isn't there a wait to inject the method will be using await/async? – RollRoll Mar 18 '16 at 21:36
2

You would replace `var response = request.GetResponse();` with `var response = await request.GetResponseAsync();` then make the whole rest of the chain async, awaiting as needed. – Scott Chamberlain Mar 18 '16 at 21:37

What is wrong using Async and Await in this WPF example?

1 Answers1