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

Question

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.

Cathar

Does IsBusy implment iNotifyPropertyChanged err does your vm do and does the property call your method for invoking the event? or is it a dp? In that case you have to change it on the UI thread. It's just that it's a typical error dealing with this. – Foreigner

Answer 1

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.

Siderite

Answer 2

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

No, RelayCommand will work as desired here.

What exactly is the current code doing regarding asynchrony?

What happenes is that when overload resolution kicks in at compile time, it chooses the overload that takes an Action, which means your method is translated into async void and that is why your code compiles.

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

There are implementations of async delegate commands. You can find one here. One important thing to note is exception handling. In the case of an unhandeled exception inside an async ICommand which is bound to a WPF control, the exception would propogate to the binder and go unhandeled and unnoticed.

Sims

Answer 3

I don't think you need an async version of relay command. Your implementation looks OK. Does it work?

If you want to test whether the body is running async, add an await task.delay(20000) and see if the UI remains responsive whilst the command is running.

Stool

Answer 4

No. Set IsAsync on the binding. Like this:

<Button Command="{Binding DoCommand, IsAsync=True}" />

Jonijonie