Implementing ICommand such that only one method may run at a time, others silently fail?

Question

I've been trying to implement ICommand in such a way that while the method it is executing is running, all other methods executed with this same type of command will fail (i.e. there is a static IsBusy variable that causes execute to return).

One caveat is that the methods passed in to the Command constructor can either be synchronous or asynchronous.

I believe the problem I am having is that the calling method continues before the callee is finished, thus setting IsBusy back to false and allowing other methods to be run.

I am having trouble doing this, and I know I'm probably doing something really dumb.

Attempted Code:

using System;
using System.Windows.Input;
using System.Threading.Tasks;

namespace DobJenkins
{
    public class ExclusiveCommand : ICommand
    {


        private Command backingCommand;
        private static bool IsBusy = false;
        private Action action;

        public ExclusiveCommand(Action a) {
            action = a;
            Action guardedAction = (Action)WrapActionWithGuard;
            backingCommand = new Command (guardedAction);
            backingCommand.CanExecuteChanged += BackingCommand_CanExecuteChanged;
        }

        void BackingCommand_CanExecuteChanged (object sender, EventArgs e)
        {
            var ev = CanExecuteChanged;
            if (ev != null) {
                ev (this, e);
            }
        }

        public void ChangeCanExecute() {
            backingCommand.ChangeCanExecute();
        }

        #region ICommand implementation

        public event EventHandler CanExecuteChanged;

        public bool CanExecute (object parameter)
        {
            return backingCommand.CanExecute (parameter);
        }

        public async void Execute (object parameter)
        {
//            if (IsBusy) {
//                return;
//            }
//
//            IsBusy = true;
//
//            await AsyncWrapper(parameter).ContinueWith(_ => IsBusy=false);

            //await AsyncWrapper(parameter);
            //IsBusy = false;
            backingCommand.Execute(parameter);
        }

        public async Task AsyncWrapper(object parameter)
        {
            backingCommand.Execute (parameter);
        }

        private void WrapActionWithGuard() {
            if (IsBusy) {
                return;
            }
            IsBusy = true;

            action.Invoke ();

            IsBusy = false;
        }

        #endregion
    }
}

Command's interface: http://i.stack.imgur.com/YfXoM.png

Attempted solutions: As you can see I tied a few different ways. I tried just simply using the logic first, and using my backing command to execute the method. Then I thought wrapping the method in an async method would allow me to do .continuewith or at least await it, but that didn't work either. Then I tried wrapping it in the logic and giving that to my backing command.

The problem:

If I pass an async method into my command, the async method gets executed fine until it hits an 'await' at which point it cedes control to the command and sets IsBusy to false.

The behavior I want is that IsBusy remains true until all of the async or sync method are done. I don't want control ceded to set IsBusy to false.

Command is a class provided by Xamarin.Forms which I don't know how I'd get the code for, but I will post the interface of it above. — Dob Jenkins

Stephen Cleary Stephen Cleary · Accepted Answer · 2015-06-02T11:50:39

I recommend that you introduce an IAsyncCommand interface and require that all asynchronous commands inherit from that interface:

public interface IAsyncCommand : ICommand
{
  Task ExecuteAsync(object parameter);
}

This enables you to asynchronously consume commands. Note that every asynchronous implementation of ICommand.Execute should be:

async void ICommand.Execute(object parameter)
{
  await ExecuteAsync(parameter);
}

Then it's pretty straightforward to implement your wrapper command:

public async Task ExecuteAsync(object parameter)
{
  if (IsBusy)
    return;
  IsBusy = true;

  var asyncCommand = backingCommand as IAsyncCommand;
  if (asyncCommand != null)
    await asyncCommand.ExecuteAsync(parameter);
  else
    backingCommand.Execute(parameter);

  IsBusy = false;
}

Note that async void should be avoided, because (as you have discovered), it's extremely difficult to consume / tell when it has completed / detect exceptions / unit test.

I have an MSDN article on async commands that you may find helpful. And on a side note, the current implementation of CanExecuteChanged in your code is incorrect; if you're delegating CanExecute, you should probably delegate CanExecuteChanged as well (though I think a more appropriate implementation would be using IsBusy).

Implementing ICommand such that only one method may run at a time, others silently fail?

1 Answers