4

I have a screen that retrieves information from a remote server and displays that information to the user. I would like this information to be updated when the screen is displayed (requiring no additional user interaction).

public partial class MyPage : ContentPage
{
    protected override async void OnAppearing()
    {
        base.OnAppearing();
        try
        {
            MyLabel.Text = (await GoGetTheData ()).Stuff;
        }
        catch (Exception e)
        {
            MyLabel.Text = "Narg";
        }
    }
}

Is this acceptable? I've seen all the discussions about the evils of "async void", and how it should only be used in event handlers, and this isn't quite an event handler.

Is the framework guaranteed to be OK in this case, since I (1) called base.OnAppearing() before getting all asynchronous, and (2) caught my exceptions?

Betty Crokker
  • 3,001
  • 6
  • 34
  • 68
  • How is this method called? – David L Mar 09 '17 at 18:21
  • It's acceptabled as these overrides are not much different from event handlers: they exist to notify an instance of a derived class about certain events. You should however keep track of the pending tasks like `GoGetTheData`. What if `OnAppearing` gets called again before the previous task has completed? Think of it as of asynchonous re-entrancy, if you like. – noseratio Mar 09 '17 at 21:08
  • I updated my code to indicate this is an override of 'OnAppearing()' which is initially define in Xamarin.Forms.Page. I know it's generally bad form to update code after people have answered, but I believe in this case my updated code in no way invalidates any of the answers or other discussion. – Betty Crokker Mar 09 '17 at 21:09

4 Answers4

12

This is acceptable. Support for async on these framework overrides were added for a reason. There are evils of "async void" in general, but this case doesn't apply.

One note however is that you should ensure that your OnAppearing(Forms), OnCreate(Android), and ViewDidLoad(iOS) methods return as soon as possible so your users have a pleasant experience.

In fact you will see this exact syntax in plenty of Forms samples: https://github.com/xamarin/xamarin-forms-samples/search?utf8=%E2%9C%93&q=async+void+OnAppearing

Jon Douglas
  • 13,006
  • 4
  • 38
  • 51
1

Assuming that this is an override of the protected method that calls the Appearing event for the page, then yes this is fine.

This method effectively is an event, it is the method that is called right before the event is raised.

EDIT: As @apineda point out in comments, the below is not actually the case.

Just note that your async stuff will be done after any event handlers for Appearing have run, since those are invoked in base.OnAppearing().

Brandon Kramer
  • 1,098
  • 6
  • 7
  • Unfortunately this doesn't seems to be case. OnAppearing is just an empty virtual method in the parent object of the ContentPage. The Appearing event handler and this method are called individually in another event of the Page. Link: https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Core/Page.cs#L184 – pinedax Mar 09 '17 at 20:35
  • @apineda Thanks for noticing that. I have edited my answer. – Brandon Kramer Mar 09 '17 at 20:41
-1

In cases where you are dealing with events (OnAppear essentiallt the callback for Appear) it's totally fine.

Return Types An async method should return a Task, Task or void.

Specify the Task return type if the method does not return any other value.

Specify Task if the method needs to return a value, where TResult is the type being returned (such as an int, for example).

The void return type is used mainly for event handlers which require it. Code that calls void-returning asynchronous methods can’t await on the result.

In short use task when possible but in cases related to event handlers just use void.

If you were needing to call async otherwise then you could use a TaskCompletionSource as per this question

Is it possible to await an event instead of another async method?

private TaskCompletionSource<object> continueClicked;

private async void Button_Click_1(object sender, RoutedEventArgs e) 
{
  // Note: You probably want to disable this button while "in progress" so the
  //  user can't click it twice.
  await GetResults();
  // And re-enable the button here, possibly in a finally block.
}

private async Task GetResults()
{ 
  // Do lot of complex stuff that takes a long time
  // (e.g. contact some web services)

  // Wait for the user to click Continue.
  continueClicked = new TaskCompletionSource<object>();
  buttonContinue.Visibility = Visibility.Visible;
  await continueClicked.Task;
  buttonContinue.Visibility = Visibility.Collapsed;

  // More work...
}

private void buttonContinue_Click(object sender, RoutedEventArgs e)
{
  if (continueClicked != null)
    continueClicked.TrySetResult(null);
}

References

https://developer.xamarin.com/guides/cross-platform/advanced/async_support_overview/

https://msdn.microsoft.com/en-us/magazine/jj991977.aspx

https://developer.xamarin.com/guides/ios/user_interface/controls/part_2_-_working_with_the_ui_thread/

Community
  • 1
  • 1
Terrance
  • 11,764
  • 4
  • 54
  • 80
  • 1
    He stated in his question that he knows that it is mainly used for event handlers. You are basically reiterating what he said he already knows. – Brandon Kramer Mar 09 '17 at 18:31
  • This is a very detailed answer, but he is just asking if his specific use is acceptable. The method he has shown appears to be the method that invokes the `Appearing` event for a `Page`, and as such is essentially an event itself. Which means that it should be fine to use async void here. – Brandon Kramer Mar 09 '17 at 18:44
  • I said that in the first revision "In short use task when possible but in cases related to event handlers just use void." FYI OnAppearing is an event callback. – Terrance Mar 09 '17 at 18:55
-3

I use this way

protected override OnAppearing(){
    base.OnAppearing();

    Task.Run(async()=>{
        try
        {
            // Use VM and Binding... 
            MyVM.Data = (await GoGetTheData ()).Stuff;
        }
        catch (Exception e)
        {

            // Here I think you should use Device.BeginInvokeOnMainThread();
            MyVm.Data = "Narg";
        }
    });
}

and somewhere...

MyText.BindingContext = MyVm;
MyText.SetBinding(Label.TextProperty, "Data");
Alessandro Caliaro
  • 5,623
  • 7
  • 27
  • 52
  • 1
    This will throw a cross thread exception because you cannot access UI objects from background threads... – Brandon Kramer Mar 09 '17 at 18:26
  • 1
    Also, it is pretty much the same as an async void method as far as consequences of using it are concerned. – Brandon Kramer Mar 09 '17 at 18:27
  • The best choice is to use "Binding". He can also try using Device.BeginInvokeOnMainThread(); but I think the UI could freeze – Alessandro Caliaro Mar 09 '17 at 18:27
  • The ui will not freeze in either case. Task.Run is not for use on already async methods. It is used to push CPU bound work to a background thread to avoid freezing the main thread. – Brandon Kramer Mar 09 '17 at 18:30