11

i want to start a task when a relay command is called, however i want to disable the button as long as that task is running

take this example

private ICommand update;
public ICommand Update
        {
            get
            {
                if (update == null)
                {
                    update = new RelayCommand(
                        param => Task.Factory.StartNew(()=> StartUpdate()),
                        param => true); //true means the button will always be enabled
                }
                return update;
            }
        }

what is the best way to check if that task is running?

here is my solution but not sure if its the best way

class Vm : ObservableObject 
    {

        Task T;
        public Vm()
        {
            T = new Task(() => doStuff());
        }

        private ICommand myCommand;
        public ICommand MyCommand
        {
            get { return myCommand ?? (myCommand = new RelayCommand( p => { T = new Task(() => doStuff()); T.Start(); }, p => T.Status != TaskStatus.Running)); }
        }


        private void doStuff()
        {
            System.Threading.Thread.Sleep(5000);
        }

    }

Update : Every answer here works fine, but still they dont agree with each other, and i just reached a 100 reputation , i start a bounty whenever i reach 100, so what i am looking for is an implementation for an optimal non memory leaking asynchronous RelayCommand that executes within a task in .net 4.0

FPGA
  • 3,525
  • 10
  • 44
  • 73
  • @MerickOWA it wont work with Task.IsCompleted, using `T.Status != TaskStatus.Running` is the best option – FPGA Nov 24 '13 at 04:35

5 Answers5

23

I strongly recommend that you avoid new Task as well as Task.Factory.StartNew. The proper way to start an asynchronous task on a background thread is Task.Run.

You can create an asynchronous RelayCommand easily using this pattern:

private bool updateInProgress;
private ICommand update;
public ICommand Update
{
  get
  {
    if (update == null)
    {
      update = new RelayCommand(
          async () =>
          {
            updateInProgress = true;
            Update.RaiseCanExecuteChanged();

            await Task.Run(() => StartUpdate());

            updateInProgress = false;
            Update.RaiseCanExecuteChanged();
          },
          () => !updateInProgress);
    }
    return update;
  }
}
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • When you are already specifying async lambda, why you are doing await on Task.Run? What is point of Task.Run anyway if RelayCommand accepts async lambda, it is supposed to execute asynchronously. – Akash Kava Dec 14 '13 at 19:00
  • 1
    `RelayCommand` does not accept an `async` lambda (i.e., `Func`); the lambda is there to wrap the `async` code into an `async void` (i.e., `Action`). The `Task.Run` is only present because the op's original code was also running `StartUpdate` on a background thread; in the ideal situation (`async` all the way), the `Task.Run` would not be necessary. – Stephen Cleary Dec 14 '13 at 19:20
  • Then what is need to mark it as async? Couldn't you call Raise Event inside Task.Run by wrapping Raise Event inside Dispatcher's BeginInvoke? – Akash Kava Dec 14 '13 at 20:08
  • Aside from the fact that direct use of `Dispatcher` is a code smell, I don't think it will work the way you're thinking. You need to have an `async void` method at some point. – Stephen Cleary Dec 14 '13 at 22:41
8

I think, you can use this implementation of AsyncCommand.

public class AsyncCommand : ICommand, IDisposable
{
    private readonly BackgroundWorker _backgroundWorker = new BackgroundWorker {WorkerSupportsCancellation = true};
    private readonly Func<bool> _canExecute;

    public AsyncCommand(Action action, Func<bool> canExecute = null, Action<object> completed = null,
                        Action<Exception> error = null)
    {
        _backgroundWorker.DoWork += (s, e) =>
            {
                CommandManager.InvalidateRequerySuggested();
                action();
            };

        _backgroundWorker.RunWorkerCompleted += (s, e) =>
            {
                if (completed != null && e.Error == null)
                    completed(e.Result);

                if (error != null && e.Error != null)
                    error(e.Error);

                CommandManager.InvalidateRequerySuggested();
            };

        _canExecute = canExecute;
    }

    public void Cancel()
    {
        if (_backgroundWorker.IsBusy)
            _backgroundWorker.CancelAsync();
    }

    public bool CanExecute(object parameter)
    {
        return _canExecute == null
                   ? !_backgroundWorker.IsBusy
                   : !_backgroundWorker.IsBusy && _canExecute();
    }

    public void Execute(object parameter)
    {
        _backgroundWorker.RunWorkerAsync();
    }

    public event EventHandler CanExecuteChanged
    {
        add { CommandManager.RequerySuggested += value; }
        remove { CommandManager.RequerySuggested -= value; }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            if (_backgroundWorker != null)
                _backgroundWorker.Dispose();
        }
    }
}
Vladimir Almaev
  • 2,358
  • 1
  • 28
  • 38
  • Does BackgroundWorker marshal to the UI thread or to whatever thread it started on? Do you need to marshal back to the UI thread? – Tom Padilla Nov 21 '14 at 16:18
3

So your solution to use RelayCommand almost works. The problem is that the UI won't immediately update after the task finishes running. This is because something needs to trigger the ICommand's CanExecuteChanged event in order for the UI to properly update.

One way to solve this problem is by creating a new kind of ICommand. For example:

  class AsyncRelayCommand : ICommand
  {
    private Func<object, Task> _action;
    private Task _task;

    public AsyncRelayCommand(Func<object,Task> action)
    {
      _action = action;
    }

    public bool CanExecute(object parameter)
    {
      return _task == null || _task.IsCompleted;
    }

    public event EventHandler CanExecuteChanged;

    public async void Execute(object parameter)
    {
      _task = _action(parameter);
      OnCanExecuteChanged();
      await _task;
      OnCanExecuteChanged();
    }

    private void OnCanExecuteChanged()
    {
      var handler = this.CanExecuteChanged;
      if (handler != null)
        handler(this, EventArgs.Empty);
    }
  }

Now your view model can do something like the following

private ICommand myCommand;
public ICommand MyCommand
{
  get { return myCommand ?? (myCommand = new AsyncRelayCommand(p => Task.Factory.StartNew(doStuff))); }
}

private void doStuff()
{
  System.Threading.Thread.Sleep(5000);
}

Or you could make your doStuff function an "async" function like so

private ICommand myCommand2;
public ICommand MyCommand2
{
  get { return myCommand2 ?? (myCommand2 = new AsyncRelayCommand(p => doStuff2())); }
}
private async Task doStuff2()
{
  await Task.Delay(5000);
}
MerickOWA
  • 7,453
  • 1
  • 35
  • 56
  • Two problems: 1) [`StartNew` is dangerous (as I explain on my blog)](http://blog.stephencleary.com/2013/08/startnew-is-dangerous.html), and 2) Your implementation of `CanExecuteChanged` [leaks memory](http://blogs.msdn.com/b/nathannesbit/archive/2009/05/29/wpf-icommandsource-implementations-leak-memory.aspx). – Stephen Cleary Nov 26 '13 at 01:22
  • @StephenCleary 1) My implementation of AsyncRelayCommand doesn't use Task.StartNew, I was merely adapting the given example code to make it as understandable as possible. 2) From what I read, this is [fixed](http://connect.microsoft.com/VisualStudio/feedback/details/713892/wpf-icommand-canexecutechanged-memory-leak) in 4.5 and is focused on changing the controls, or ICommandSource, not on changing the implementations of ICommand. – MerickOWA Nov 26 '13 at 23:36
0

You could have a static variable IsRunning, which you can set to True when your task starts, to false when it finishes, and just bind that enabled button to the state of the IsRunning

Noctis
  • 11,507
  • 3
  • 43
  • 82
  • i did that already, but is there no way without extra variables? – FPGA Nov 24 '13 at 04:19
  • well, someone needs to know about something ... how would you do it otherwise? :) – Noctis Nov 24 '13 at 04:20
  • maybe by making the task global?, it would be disturbing because i have alot of commands and i need the buttons to be disabled as long as the related task is running – FPGA Nov 24 '13 at 04:21
  • A static flag should only be used in the simplest of scenarios, this approach very quickly falls apart when things start getting more complex. – slugster Nov 24 '13 at 04:31
  • @slugster that sounds like a simple scenario to me. Otherwise, what is it you're suggesting? seems like I got a downvote, and OP never got an alternative ... – Noctis Nov 24 '13 at 09:36
0

I am trying to avoid Prism library to keep my control simple as possible from point of view of mount of reference assemblies and I ended up with this solution

_cmd = new RelayCommand(async delegate
{
   await Task.Run(() => <YourMethod>());
}, delegate { return !IsInProgress; }) );

Seems to be working well. (if you don't need to pass commandParameter in). Unfortunately this is still a problem.

RelayCommand class inherits from ICommand

public class RelayCommand : ICommand
{
    private Action<object> _execute;

    private Predicate<object> _canExecute;

    private event EventHandler CanExecuteChangedInternal;

    public RelayCommand(Action<object> execute)
        : this(execute, DefaultCanExecute)
    {
    }

    public RelayCommand(Action<object> execute, Predicate<object> canExecute)
    {
        if (execute == null)
        {
            throw new ArgumentNullException("execute");
        }

        if (canExecute == null)
        {
            throw new ArgumentNullException("canExecute");
        }

        _execute = execute;
        _canExecute = canExecute;
    }

    public event EventHandler CanExecuteChanged
    {
        add
        {
            CommandManager.RequerySuggested += value;
            CanExecuteChangedInternal += value;
        }

        remove
        {
            CommandManager.RequerySuggested -= value;
            CanExecuteChangedInternal -= value;
        }
    }

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

    public void Execute(object parameter)
    {
        _execute(parameter);
    }

    public void OnCanExecuteChanged()
    {
        EventHandler handler = CanExecuteChangedInternal;
        if (handler != null)
        {
            //DispatcherHelper.BeginInvokeOnUIThread(() => handler.Invoke(this, EventArgs.Empty));
            handler.Invoke(this, EventArgs.Empty);
        }
    }

    public void Destroy()
    {
        _canExecute = _ => false;
        _execute = _ => { return; };
    }

    private static bool DefaultCanExecute(object parameter)
    {
        return true;
    }
}
st35ly
  • 1,215
  • 18
  • 24