8

I've learned to lazy load properties in my repository. Now I'd like to do that, but I also need to load something from a web page (using Httpclient) which means my property will be async.

public async Task<List<NewsModel>> News
{
    get
    {
        if (_news == null)
        {
            CacheProvider cache = new CacheProvider();
            object cachedNews = cache.Get("news");

            if (cachedNews == null)
            {
                var client = new HttpClient();
                // await HttpResponse
            }

        }
        return _news;
    }

    set
    {
        _news = value;
    }
}

However, visual studio is telling me that

"The modifier async is not valid for this item"

while highlighting the word "News" in the first line.

Is it possible to do this? Or do I have to write a separate method?

Saturnix
  • 10,130
  • 17
  • 64
  • 120
  • I take it that you've got the using statement for System.Threading.Tasks? Also, which .NET framework is this targeting? Is it Silverlight? -- Never mind, just saw that this is a property. You'll want to convert this into methods. – Ben H Jul 31 '13 at 16:21
  • It seems to me like the property `News` would intuitively have the type `List`, the `Task<>` decoration doesn't seem natural here. Why do you feel that you need to expose the `Task<>` nature? – Andrew Coonce Jul 31 '13 at 16:21
  • @AndrewCoonce Actually, if he wants to write an async *method*, he would definitely have to write it as returning a task. – Lasse V. Karlsen Jul 31 '13 at 16:22
  • Because visual studio told me to do that :( First time I deal with async in C#. – Saturnix Jul 31 '13 at 16:22
  • @LasseV.Karlsen: Yes, but this isn't a method... this is a property. I guess my concern is that the end-user isn't expecting side-effects, asynchronous operation, etc... they're expecting an enumeration of news items. – Andrew Coonce Jul 31 '13 at 16:25
  • Possible duplicate of [caching the result from a \[n async\] factory method iff it doesn't throw](http://stackoverflow.com/questions/33872588/caching-the-result-from-a-n-async-factory-method-iff-it-doesnt-throw) – Ruben Bartelink Nov 26 '15 at 20:30

4 Answers4

14

Asynchronous properties are not supported. I describe a number of workarounds on my blog.

In your case, it sounds like asynchronous lazy initialization would be a good solution (also described on my blog).

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
9

First of all, properties with side-effects are usually not all that good.

In this case, simply reading this property would kick off a thread, some network traffic, and some processing on a remote server.

This should be a method, not a property.

Secondly, the compiler is right, properties are not allowed to be asynchronous. Now, you can definitely write a property that returns an asynchronous task, but you're not allowed to use the async keyword. Basically just take away the async keyword from the property declaration.

But the fact that async is not legal on properties is another clue that you should write a method, not a property.

Note The async keyword is not actually required in the code you've posted, since you're not actually using the await keyword in there. As such, you can simply remove the async keyword altogether, since it is not required. In fact, if you were to change this over to a method, the compiler will tell you that it is unnecessary since you're not using await. (Removed after OP edited the question.)

Lasse V. Karlsen
  • 380,855
  • 102
  • 628
  • 825
  • oh wait! I am using await! httpclient will use await as the guy here did: http://stackoverflow.com/questions/7929013/making-a-curl-call-in-c-sharp – Saturnix Jul 31 '13 at 16:24
  • If I remove the 'await' I have to wrap HttpResponseMessage in a task - which means I have to write an 'await' somewhere in the code... – Saturnix Jul 31 '13 at 16:25
  • Well, you still cannot write properties using `async`, so `await` has to go as well. – Lasse V. Karlsen Jul 31 '13 at 16:27
  • So should I delete the ENTIRE property and write it as a method instead? Many thanks for your help btw :) – Saturnix Jul 31 '13 at 16:28
  • Yes. That is what you should do. – Lasse V. Karlsen Jul 31 '13 at 16:29
  • Ok, last question: should then the method be "async" or return a "Task<>"? What do you advise? – Saturnix Jul 31 '13 at 16:31
  • The op's code does not start another thread. Also, an `async` method is not a good semantic match either because the op only wishes the network request to happen once. – Stephen Cleary Jul 31 '13 at 16:32
  • 2
    _"First of all, properties with side-effects are usually not all that good"_ - well no, that's how **proxy classes** with **on-demand/lazy-load** loading work such as in _ORMs_ –  Nov 13 '14 at 01:29
  • 2
    I agree with "properties with side-effects are usually not all that good" but I don't think it is applicable here. Properties should not change the logical state of the system. I don't think loading data is a side-effect. Querying a property is asking a question. Working to answer the question is an expected effect. – Hypnovirus Jul 24 '15 at 14:42
4

You can use a Lazy property that returns a Task:

class MyClass
{
    readonly Lazy<Task<string>> _text;

    public MyClass()
    {
        _text = new Lazy<Task<string>>(async () =>
        {
            //... await something
            return "Hello!"
        });
    }

    async Task DoSomething()
    {
       var text = await _text.Value;
       //...
    }
}
Artem
  • 1,773
  • 12
  • 30
1

I think this question is related.

In short - async properties are not supported.

Community
  • 1
  • 1
MBender
  • 5,395
  • 1
  • 42
  • 69