JD Davis
JD Davis

Reputation: 3720

MVVM Light Call async method on property changed?

So I've been doing quite a bit of digging and I haven't been able to find any real definitive answer to this anywhere.

I'm writing an application using MVVM Light and WPF. I have a service that is injected into my ViewModel that checks the validity of a certain property that is set. The service makes a web-call, and it is asynchronous. The call does not need to halt execution of the application, and it's solely to provide visual feedback to the user as to the validity of the entered value.

As such, I've managed to hack something together to get it to work, but it feels somewhat hackey.

What is the proper way to execute an asynchronous method on a property change without resorting to something like async void?

Here's what I currently have.

    public string OmmaLicenseNumber
    {
        get => _ommaLicenseNumber;
        set
        {
            Set(() => OmmaLicenseNumber, ref _ommaLicenseNumber, value);
            Patient.OmmaLicenseNumber = value;

            var _ = CheckLicenseValid();
        }
    }

    private async Task CheckLicenseValid()
    {
        var valid = await _licenseValidationService.IsValidAsync(OmmaLicenseNumber);

        // We don't want the UI piece showing up prematurely. Need 2 properties for this;
        LicenseValid = valid;
        LicenseInvalid = !valid;
    }

If I simply attempt to call .Result on The async method, it results in a deadlock that requires an application restart to fix. And while what I have works, I'm not really a fan. What are my other options?

Upvotes: 5

Views: 1798

Answers (3)

TheGeneral
TheGeneral

Reputation: 81543

The issue is here not how to run an async task unobserved, it's what do you do with the exceptions. I say this because they might turn up when the task gets cleaned up.

Ideally, just show the next reader what they are getting. Since you are against the use of async void.

Option 1

// running an async method unobserved 
Task.Run(async () =>
{
   try
   {
      await DoSomethingAsync();
   }
   catch (Exception e)
   {
       // Deal with unobserved exceptions 
       // or there will be dragons
   }  
});

Note : This is technically offloading (it will lose the context BEWARE) and the async lamda makes it an async void anyway, however you will have to deal with the exceptions in any case though

Option 2 and more controversially:

public async void DoSomethingFireAndForget()
{
   try
   {
      await DoSomethingAsync();
   }
   catch (Exception e)
   {
      // Deal with unobserved exceptions 
      // or the will be dragons
   }  
}

Option 3 best of both worlds:

Note : Use whatever plumbing you need to observe the exceptions, ie Action etc

public static class TaskUtils
{

#pragma warning disable RECS0165 // Asynchronous methods should return a Task instead of void
   public static async void FireAndForgetSafeAsync(this Task task,  Action<Exception> onErrors = null)
#pragma warning restore RECS0165 // Asynchronous methods should return a Task instead of void
   {
      try
      {
         await task;
      }
      catch (Exception ex)
      {
         onErrors?.Invoke(ex);
      }
   }
}

Upvotes: 2

Ackdari
Ackdari

Reputation: 3498

Take a look at the NotifyTask-Class written by Stephen Cleary.

It is a good way to handle async operations in constructors and properties.

You could refactor your code to be like:

private NotifyTask _OmmaLicenseNumberChangedNotifyTask
    = NotifyTask.Create(Task.CompletedTask);
public NotifyTask OmmaLicenseNumberChangedNotifyTask
{
    get => this._OmmaLicenseNumberChangedNotifyTask;
    set
    {
        if (value != null)
        {
            this._OmmaLicenseNumberChangedNotifyTask = value;
            OnPropertyChanged("OmmaLicenseNumberChangedNotifyTask");
        }
    }
}

public string OmmaLicenseNumber
{
    get => _ommaLicenseNumber;
    set
    {
        Set(() => OmmaLicenseNumber, ref _ommaLicenseNumber, value);
        Patient.OmmaLicenseNumber = value;

        OmmaLicenseNumberChangedNotifyTask = NotifyTask.Create(CheckLicenseValid());
    }
}

Then you can forget about the Task or you can bind elements of your UI to the properties of the NotifyTask, like the IsCompleted to make something visible only if the Task has completet.

Upvotes: 2

Nkosi
Nkosi

Reputation: 247423

Event handlers allow for the use of async void

Reference Async/Await - Best Practices in Asynchronous Programming

public string OmmaLicenseNumber {
    get => _ommaLicenseNumber;
    set {
        Set(() => OmmaLicenseNumber, ref _ommaLicenseNumber, value);
        Patient.OmmaLicenseNumber = value;
        //Assuming event already subscribed 
        //i.e. OmmaLicenseNumberChanged += OmmaLicenseNumberChanged;
        OmmaLicenseNumberChanged(this, 
            new LicenseNumberEventArgs { LicenseNumber = value }); //Raise event
    }
}

private event EventHandler<LicenseNumberEventArgs> OmmaLicenseNumberChanged = delegate { };
private async void OnOmmaLicenseNumberChanged(object sender, LicenseNumberEventArgs args) {
    await CheckLicenseValid(args.LicenseNumber); //<-- await async method call
}

private async Task CheckLicenseValid(string licenseNumber) {
    var valid = await _licenseValidationService.IsValidAsync(licenseNumber);

    // We don't want the UI piece showing up prematurely. Need 2 properties for this;
    LicenseValid = valid;
    LicenseInvalid = !valid;
}

//...

public class LicenseNumberEventArgs : EventArgs {
    public string LicenseNumber { get; set; }
}

Do I think this to be a little heavy handed?

Yes. This is just meant as an example to show that it is doable.

Can this be refactored to some simpler helper/utility method call?

Yes. Could look a lot like an awaitable callback pattern using expressions to get value to validate

Upvotes: 4

Related Questions