2

Both of these work, but the first results in a warning that execution continues after the called to GetDateRanges and that I should use await. The call is in a constructor so I don't think I can do that.

Given that the result is the same, is there some other significant difference between these code snippets? Is the first holding up the main thread and I haven't noticed?

public MainPage(IDateRangeRepository dateRangeRepository)
{
    GetDateRanges();
}

async Task GetDateRanges()
{
    var ranges = await dateRangeRepository.FetchDateRangesAsync(
        DateTime.Now, DateTime.Now);

    calendar.DateRanges = ranges;
}

With Task.Run... 

public MainPage(IDateRangeRepository dateRangeRepository)
{
   Task.Run(GetDateRanges);
}

async Task GetDateRanges()
{
   var ranges = await dateRangeRepository.FetchDateRangesAsync(
      DateTime.Now, DateTime.Now);

   Device.BeginInvokeOnMainThread(() =>
   {
      calendar.DateRanges = ranges;
   });
}
Servy
  • 202,030
  • 26
  • 332
  • 449
Ian Warburton
  • 15,170
  • 23
  • 107
  • 189
  • The calendar property is bound to the result of the lengthy task. – Ian Warburton Oct 20 '17 at 12:36
  • What is `calendar.DateRanges` and how is code that depends on it having been set going to know when that has happened? – Damien_The_Unbeliever Oct 20 '17 at 12:36
  • `calendar.DateRanges` is a collection that's bound to the calendar. Any dependent code works with whatever is in the collection - even if it's empty. But, I could add a notification upon change if necessary. – Ian Warburton Oct 20 '17 at 12:39
  • 1
    I would use the second with `Task.Run`... If that takes a while to complete I would add a notification to the user that "Data is Loading" – u8it Oct 20 '17 at 12:40
  • 1
    Generally speaking, it's a code smell if an object's constructor starts a long-running operation causing even some **background threads** to be used. A constructor has to be as simple as possible. – dymanoid Oct 20 '17 at 12:40
  • @dymanoid, that's reasonable, unless the constructor is a main entry point. Then you've got to do what you've got to do to get loaded anyway. – u8it Oct 20 '17 at 12:42
  • @dymanoid Yes, I ought to put the call in Xamarin Form's `OnAppearing` or something. But that's not an async method either - so the issue remains. – Ian Warburton Oct 20 '17 at 12:42
  • 1
    @u8it, a constructor is not the entry point into a program. It's `static void Main()` in .NET. – dymanoid Oct 20 '17 at 12:43
  • @dymanoid Good point. – Ian Warburton Oct 20 '17 at 12:47
  • @dymanoid, yea I realize that, I guess I'm suggesting that if you're launching a main form with an application message loop from `Main()`, then for all intents and purposes it's practical to treat your main form's constructor as your application's entry point... maybe that doesn't apply to the op's situation anyway, just a thought regarding thread spawning out of a constructor though. – u8it Oct 20 '17 at 12:52
  • In fact, I'm not sure how much I agree that thread spawning out of a constructor should be viewed as code smell at all in general. What if you have a Form that utilizes a background worker, timer, or FileSystemWatcher that needs to start handling operations right away while you want your form to be responsive? It seems like there are plenty of situations where the most simple thing to do is start your parallel operations out of your constructor while it goes ahead and gets your GUI up and responsive. If the constructor and/or object has parallel tasks, then thread spawning makes sense. – u8it Oct 20 '17 at 13:04
  • For my money, if you dont need the result to be filled instantly and you just expect it to finish later then `Task.Run` is a good option. This is only true if you don't care about execution order. – FCin Oct 20 '17 at 13:08
  • 1
    `Task.Run` here accomplishes literally nothing other than to slow down your code a bit. It will still behave the same way otherwise, as the operation is already asynchronous. Of course, *neither* solution actually works effectively, as both return an improperly constructed object and provide the caller with no way of knowing when the object will be properly constructed. – Servy Oct 20 '17 at 13:19
  • 1
    all XF pages have an async OnAppearing method that can be used instead – Jason Oct 20 '17 at 13:28
  • 1
    @Servy, I really don't think this is a duplicate of the question you marked. The question here doesn't ask about making a contrcutor `Asynch`, it asks about not awaiting an Asynch vs using a task. I find these two question more relevant to that: [Await Warning](https://stackoverflow.com/q/11147187/3546415) [Not Awaited](https://stackoverflow.com/q/14903887/3546415)... and there might be a Xamarin specific answer here being overlooked – u8it Oct 20 '17 at 13:28
  • @u8it The question specifically says that the problem they're having is that they need to have an async constructor, but can't do it, and they're wondering if `Task.Run` solves that problem. The duplicate shows them the *correct* way to solve their actual problem. – Servy Oct 20 '17 at 13:29
  • @Servy ok, correct me if I'm wrong, but I feel like there's more to this than what's covered in that QA. For instance, there's more to warning 4014 and the use of Task vs Asynch... The QA links I posted above suggest that the `Task` solution would be better in this case because it provides better exception handling over an Asynch Void... Asynch void is truly set and forget but I imagine the OP cares about a data exception in this case... also this has the Xamarin tag. – u8it Oct 20 '17 at 13:39
  • @Servy Why is the object improperly constructed? – Ian Warburton Oct 20 '17 at 13:42
  • @u8it The OP *does* have an async void method, for all intents and purposes, since they ignore the returned task. The duplicate shows numerous solutions for not simply firing and forgetting an async method, ranging from a factory method to exposing the task instead of dropping it on the floor. The use of Xamarin doesn't change anything here. – Servy Oct 20 '17 at 13:43
  • 1
    @IanWarburton Becuase you return an object that hasn't been constructed yet, provide no way for any caller to know when it *will* be properly constructed, and drop any errors you get when constructing the object on the floor, instead of being able to handle them. – Servy Oct 20 '17 at 13:43
  • @Servy, I definitely see how the QA you marked as a dup relates to this Question, but I don't see the full breadth of discussion there that this question begs for. Maybe I'm wrong about this, but exposing the Task to call a private constructor or using a Factory pattern isn't always a solution because you don't have control of callers and sometimes a public constructor is required. This is actually mentioned in the comments of that QA but never addressed the way the OP is asking about here. Also, the factory pattern is only briefly mentioned as a link in a comment. – u8it Oct 20 '17 at 13:58
  • Also, again just my thinking here, but the nature of QA is such that mutual exclusion should be based on the perspective of the question and not necessarily the overlap of the answer, since the question is typically what is searched and recognized. For instance, what did I eat for breakfast and what did I eat for dinner are different questions even if I ate the same thing. – u8it Oct 20 '17 at 13:59
  • To that point, I think the title of this question should be renamed to something like "Asynch vs Task in Constructor". – u8it Oct 20 '17 at 14:00
  • @u8it There are hundreds of discussions on the topic if you want more information, you only need to google "async constructor" if you want to see more information on the topic than you could ever possibly read. The linked dup provides more than enough for a solution, though. Again, numerous solutions were provided, so the OP has the information available to them as to what options they have and what their advantages/disadvantages are. The fact that it's only a comment that uses the word "factory method" doesn't change the fact that several answers show how to write one. – Servy Oct 20 '17 at 14:00
  • @u8it I've edited the title to more closely reflect the question asked in the body, as you requested. – Servy Oct 20 '17 at 14:01
  • @Servy I don't need more information, I'm just thinking about the OP and that it'd be more useful to have an actual answer here rather than an Easter egg hunt through another QA thread. 90% of SO answers are Googleable; it's the phrasing and curating that adds value... but I'll drop this now because it really doesn't matter that much and I trust your judgment. Thanks for changing the title, much better. – u8it Oct 20 '17 at 14:08
  • @Jason What is the async OnAppearing method called? (Although it's not right for my situation though because I want to bind on first load, not every time the page appears.) – Ian Warburton Oct 20 '17 at 14:10
  • you can easily use a bool flag to prevent loading more than once. And it's called "OnAppearing" (override the base method) – Jason Oct 20 '17 at 14:15
  • @Jason ... or use the constructor. `OnAppearing` isn't async. – Ian Warburton Oct 20 '17 at 14:18
  • 1
    yes it is, just add the async keyword – Jason Oct 20 '17 at 14:24
  • @Jason Great, thanks. But it's not in the class reference. How does the system know to call it using `await`? – Ian Warburton Oct 20 '17 at 14:29
  • @Servy The answers to that question either offer the options I've suggested or use a factory method - which I can't do because the class in question is created by the framework (thus the object is returned constructed as far as possible). I don't see a *comparison* between the alternatives I mentioned. – Ian Warburton Oct 20 '17 at 14:30

0 Answers0