8
\$\begingroup\$

Goals

  • I want to be able to call async code using the MVVM pattern
  • I want to be able to add loading animations/screens without having to add properties for each command on my viewmodels
  • I may want to be able to cancel these operations

This is what I came up with

public abstract class CommandBase : ICommand
{
    private readonly Func<bool> _canExecute;

    public CommandBase()
    {

    }

    public CommandBase(Func<bool> canExecute)
    {
        if (canExecute == null) throw new ArgumentNullException(nameof(canExecute));
        _canExecute = canExecute;
    }

    public bool CanExecute(object parameter)
    {
        return _canExecute == null || _canExecute();
    }

    public abstract void Execute(object parameter);

    public void RaiseCanExecuteChanged()
    {
        CanExecuteChanged?.Invoke(this, EventArgs.Empty);
    }

    public event EventHandler CanExecuteChanged;
}


public class AsyncCommand : CommandBase, INotifyPropertyChanged
{
    private readonly Func<CancellationToken, Task> _action;

    private CancellationTokenSource _cancellationTokenSource;

    private bool _isRunning;
    public bool IsRunning
    {
        get { return _isRunning; }
        set
        {
            _isRunning = value; 
            OnPropertyChanged();
        }
    }

    private ICommand _cancelCommand;
    public ICommand CancelCommand => _cancelCommand ?? (_cancelCommand = new RelayCommand(Cancel));

    public AsyncCommand(Func<CancellationToken, Task> action)
    {
        if (action == null) throw new ArgumentNullException(nameof(action));
        _action = action;
    }

    public AsyncCommand(Func<CancellationToken, Task> action, Func<bool> canExecute) : base(canExecute)
    {
        if (action == null) throw new ArgumentNullException(nameof(action));
        _action = action;
    }

    private void Cancel()
    {
        _cancellationTokenSource?.Cancel();
    }

    public override async void Execute(object parameter)
    {
        IsRunning = true;
        try
        {
            using (var tokenSource = new CancellationTokenSource())
            {
                _cancellationTokenSource = tokenSource;

                await ExecuteAsync(tokenSource.Token);
            }
        }
        finally
        {
            _cancellationTokenSource = null;
            IsRunning = false;
        }
    }

    private Task ExecuteAsync(CancellationToken cancellationToken)
    {
        return _action(cancellationToken);
    }

    public event PropertyChangedEventHandler PropertyChanged;

    [NotifyPropertyChangedInvocator]
    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}

It can be used like this:

<Button Content="Start" Command="{Binding UpdateDisplayTextCommand}"/>
<Button Content="Cancel" Command="{Binding UpdateDisplayTextCommand.CancelCommand}"/>
<ProgressBar IsIndeterminate="{Binding UpdateDisplayTextCommand.IsRunning}" />

Any faults on this implementation? I'm also open to ideas on features which could be added to this class.

\$\endgroup\$
6
\$\begingroup\$

You should avoid code duplication in constructors. For larger objects this practice can quickly spin out of control. Re-use existing constructors instead:

public AsyncCommand(Func<CancellationToken, Task> action) : this(action, () => true) {}

or use default parameters:

public AsyncCommand(Func<CancellationToken, Task> action, Func<bool> canExecute = null) : base(canExecute)
{
    if (action == null) throw new ArgumentNullException(nameof(action));
    _action = action;
}

public CommandBase(Func<bool> canExecute = null)
{
    //this also allows you to remove "_canExecute == null" check
    _canExecute = canExecute ?? () => true;
}

Also, public constructors in abstract class look weird. You should make those protected.

Another minor thing: you have public and private members mixed together in a way, that does not make much sense. Personally, I find code easier to follow, if it has members of the same access level grouped together. But you can use any other scheme (some people like to group ICommand property with related delegate, for example), as long as it does not look random and chaotic.

P.S. If I were to use this class, I would also want to have an overloaded constructor, which takes Action<CancellationToken>. I don't want to manually create tasks, I want you to do it for me. I'm lazy like that. :) I would also want the command to either implement IDisposable or have a public Cancel method, so I can stop async operation programmatically. I mean I could call AsyncCommand.CancelCommand.Execute(...) but it looks ugly.

\$\endgroup\$
2
\$\begingroup\$

Looks well implemented.

Just one point: If IsRunning is true, CanExecute should return false to avoid multiple execution.


The AsyncCommand looks a little bit like a mixture of a command and a view model. I think it is ok for that case. However another option could be to create a view model "AsyncOperationViewModel" with the the properties Command, CancelCommand and IsRunning.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.