28

I have a long running process in an mvvmcross viewmodel and wish to make it async (http://msdn.microsoft.com/en-us/library/vstudio/hh191443.aspx).

The async keyword is currently supported in the beta channel for Xamarin.

Below is an example of how I'm currently implementing async. The IsBusy flag ccould be bound to a UI element and display a loading message.

Is this the correct way?

public class MyModel: MvxViewModel
{
    private readonly IMyService _myService;
    private bool _isBusy;

    public bool IsBusy
    {
        get { return _isBusy; }
        set { _isBusy = value; RaisePropertyChanged(() => IsBusy); ; }
    }

    public ICommand MyCommand
    {
        get
        {
            return new MvxCommand(DoMyCommand);
        }
    }

    public MyModel(IMyService myService)
    {
        _myService = myService;
    }

    public async void DoMyCommand()
    {
        IsBusy = true;
        await Task.Factory.StartNew(() =>
            {
                _myService.LongRunningProcess();
            });
        IsBusy = false;
    }

}
svick
  • 236,525
  • 50
  • 385
  • 514
Chris Koiak
  • 1,059
  • 2
  • 11
  • 24

4 Answers4

33

You should avoid async void. When you're dealing with ICommand, you do need to use async void, but its scope should be minimized.

This modified code exposes your action as an async Task, which is unit testable and consumable from other parts of your code:

public class MyModel: MvxViewModel
{
  private readonly IMyService _myService;
  private bool _isBusy;

  public bool IsBusy
  {
    get { return _isBusy; }
    set { _isBusy = value; RaisePropertyChanged(() => IsBusy); ; }
  }

  public ICommand MyCommand
  {
    get
    {
      return new MvxCommand(async () => await DoMyCommand());
    }
  }

  public MyModel(IMyService myService)
  {
    _myService = myService;
  }

  public async Task DoMyCommand()
  {
    IsBusy = true;
    await Task.Run(() =>
    {
      _myService.LongRunningProcess();
    });
    IsBusy = false;
  }
}

Your use of IsBusy is fine; that's one common approach in asynchronous UIs.

I did change Task.Factory.StartNew to Task.Run; Task.Run is preferred in async code for reasons described by Stephen Toub.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Since I posted this I've removed the IsBusy flag and added calls to an IAlertsService. This helps centralise the loading/alert logic and removes it from individual views. – Chris Koiak Jun 19 '13 at 11:29
  • This works fine: return new MvxCommand(() => DoMyCommand()); – Chris Koiak Jun 19 '13 at 11:37
  • 3
    The `async` lambda gives you slightly different error handling. With `() => DoMyCommand()`, any exceptions from `DoMyCommand` are silently swallowed. With `async () => await DoMyCommand()`, any exceptions from `DoMyCommand` are treated as unhandled exceptions. – Stephen Cleary Jun 19 '13 at 11:45
  • @ChrisKoiak, can you give an example of how you implement and use an IAlertService in a blog post or article or just a conceptual explanation here? – Shawn Mclean Oct 06 '13 at 22:21
  • @Shawn try here http://stackoverflow.com/questions/16788164/custom-plugin-in-mvvmcross – Chris Koiak Oct 07 '13 at 07:06
  • @StephenCleary you rock! but i have a question, should we use this code when using async commands or should we need to implement https://msdn.microsoft.com/en-us/magazine/dn630647.aspx? what's the differences? Thanks! – Vackup Apr 12 '16 at 16:20
  • 1
    @Vackup: I tend to follow the simple pattern in this answer until I need something more complex. A custom "async command" can handle some use cases better: busy spinners / disabled-when-executing, cancellation commands, data binding results/errors. [My latest async command stuff is here](https://github.com/StephenCleary/Mvvm.Async/tree/master/src/Nito.Mvvm.Async). – Stephen Cleary Apr 12 '16 at 17:24
  • Thanks a lot @StephenCleary! now everything is clear! – Vackup Apr 12 '16 at 18:03
6

MvvmCross now has MvxAsyncCommand (see GitHub commit).

So instead of doing this

public ICommand MyCommand
{
  get
  {
    return new MvxCommand(async () => await DoMyCommand());
  }
}

You can do this

public ICommand MyCommand
{
  get
  {
    return new MvxAsyncCommand(DoMyCommand);
  }
}
Colin Bacon
  • 15,436
  • 7
  • 52
  • 72
4

Looks OK except I would add a try catch finally around that await.

    public async void DoMyCommand()
    {
        IsBusy = true;
        try{
            await Task.Factory.StartNew(() =>
                                        {
                _myService.LongRunningProcess();
            });
        }catch{
            //Log Exception
        }finally{
            IsBusy = false;
        }
    }

Further more I have an example on my blog using an MvxCommand with async. Very similar to your example http://deapsquatter.blogspot.com/2013/03/updating-my-mobile-apps-for-async.html

Kevin
  • 2,281
  • 1
  • 14
  • 16
1

You can also use MethodBinding plugin to avoid boiler plate code (commands), and bind your UI directly to the async method.

Besides, if you use Fody PropertyChanged, your code would look like this:

[ImplementPropertyChanged]
public class MyModel: MvxViewModel
{
    private readonly IMyService _myService;

    public bool IsBusy { get; set; }

    public MyModel(IMyService myService)
    {
        _myService = myService;
    }

    public async Task DoSomething()
    {
        IsBusy = true;
        await Task.Factory.StartNew(() =>
        {
                _myService.LongRunningProcess();
        });
        IsBusy = false;
    }
}

You can make the binding like: "Click DoSomething".

On the other hand, rather than using await Task.Factory.StartNew(), why not making _myService.LongRunningProcess async? It would look much better:

public async Task DoSomething()
{
    IsBusy = true;
    await _myService.LongRunningProcess();
    IsBusy = false;
}
xleon
  • 6,201
  • 3
  • 36
  • 52