is asynchronous version of relaycommand required in order to run async methods correctly

Coding Dude picture Coding Dude · Aug 2, 2014 · Viewed 11.9k times · Source

I have the following code defined in a viewmodel. I think that the SaveAsync of type Func<Task> is getting converted to Action since RelayCommand takes an Action not a Func<Task> but I'm not clear on the implications of that.

1) Does RelayCommand need to be replaced with an asynchronous version (RelayCommandAsync)? 2) What exactly is the current code doing regarding asynchrony? 3) What if anything could/should I change to improve/correct it?

private ICommand _saveCommand;
public ICommand SaveCommand
{
    get { return _saveCommand ?? (_saveCommand = new RelayCommand(async () => await SaveAsync(), CanSave)); }
}

public bool CanSave()
{
    return !IsBusy;
}

private async Task SaveAsync()
{
    IsBusy = true;

    try
    {
        await _service.SaveAsync(SomeProperty); 
    }
    catch ( ServiceException ex )
    {
        Message = "Oops. " + ex.ToString();
    }
    finally
    {
        IsBusy = false;
    }
}

Thanks!

EDIT: After some experimenting it does appear that the async method itself works as it should. And it did not make a difference whether async/await was included in the lambda nor whether the method was defined as async Task or async void.

However, what does not work correctly is the canExecute predicate functionality that automatically enables/disables the control binding to the command. What happens is that the button is correctly disabled while the async method runs but it is not enabled afterwards. I have to click somewhere on the window once and then it becomes enabled again.

So it appears an async version of RelayCommand is required for full functionality, i.e. so canExecute can do its thing correctly.

Answer

Stephen Cleary picture Stephen Cleary · Aug 3, 2014

1) Does RelayCommand need to be replaced with an asynchronous version (RelayCommandAsync)?

It doesn't have to be, but you should consider it.

2) What exactly is the current code doing regarding asynchrony?

It's creating an async void lambda. This is problematic because async void does not handle exceptions particularly nicely. If you do use RelayCommand with asynchronous code, then you'll definitely want to use a try/catch like the one in your code.

3) What if anything could/should I change to improve/correct it?

If this is the only async command in your code, I'd say it's fine. However, if you find that you have several async commands in your application with similar semantics, then you should consider writing a RelayCommandAsync.

There is no standard pattern (yet); I outline a few different approaches in an MSDN article. Personally, at the very least I define an IAsyncCommand in my applications, which I expose from my VMs (it's difficult to unit test an async ICommand).

However, what does not work correctly is the canExecute predicate functionality that automatically enables/disables the control binding to the command.

Assuming that RelayCommand.CanExecuteChanged is delegating to CommandManager, then you can just call CommandManager.InvalidateRequerySuggested after setting IsBusy.