Populate ObservableCollection with data from awaited Task.Run

Question

I am making Xamarin application and I have a problem with display data in my list view. So, basically I have web service from where I fetch data (that endpoint is non-async), so I don't want to block UI thread of my application, so I wrap the call to web service in Task.Run and await that task.

public class HomeDetailPageViewModel : ViewModelNavigatable
{
    public ObservableCollection<CarViewModel> cars;

    public ObservableCollection<CarViewModel> Cars
    {
        get { return this.cars; }
    }

    public HomeDetailPageViewModel(INavigationService navigationService)
        :base(navigationService)
    {
        this.cars = new ObservableCollection<CarViewModel>();
        AppearingCommand = new AsyncCommand(this.OnAppearingCommandExecuted);
    }

    public IAsyncCommand AppearingCommand { get; set; }


    public async Task OnAppearingCommandExecuted()
    {
        using (UserDialogs.Instance.Loading("Loading..."))
        {
            this.Cars.Clear();
            IEnumerable<Car> carsFromEndpoint = await Task.Run(() => CarEndpoint.GetAll(client.Context)).ConfigureAwait(false);

            Device.BeginInvokeOnMainThread(() =>
            {
                foreach (var carFromEndpoint in carsFromEndpoint.ToList())
                    this.Cars.Add(new CarViewModel
                    {
                        Manufacturer = carFromEndpoint.Code,
                        Model = carFromEndpoint.Name,
                        Price = carFromEndpoint.Price,
                        Year = carFromEndpoint.Year
                    });
            });
        }
    }
}

As I said CarEndpoint.GetAll(client.Context) is synchronous endpoint. If I use:

Task.Run(() => CarEndpoint.GetAll(client.Context)).Result or

CarEndpoint.GetAll(client.Context)

everything works as expected but that is not acceptable because it's block UI thread until Task is finished. I know it is not a good idea to use Task.Run to make fake asynchronous calls, but I don't see another way to keep app responsive, because I cannot change the endpoint of web service.

Thank you for the answer. Cheers :)

Answer 1

When you're using Async commands in MVVM, the main problem is NotSupportedException while modifying the ObservableCollection. All the rest don't cause any issues if you're careful with concurrency.

Here's a class representing ObservableCollection for use from multiple threads which forwards all actions to SynchronizationsContext of the Thread where it was constructed.

Just use it instead of ObservableCollection (not mine, grabbed from GitHub)

public class AsyncObservableCollection<T> : ObservableCollection<T>
{
    private readonly SynchronizationContext _synchronizationContext = SynchronizationContext.Current;

    public AsyncObservableCollection() : base() { }
    public AsyncObservableCollection(IEnumerable<T> collection) : base(collection) { }
    public AsyncObservableCollection(List<T> list) : base(list) { }

    private void ExecuteOnSyncContext(Action action)
    {
        if (SynchronizationContext.Current == _synchronizationContext)
            action();
        else
            _synchronizationContext.Send(_ => action(), null);
    }

    protected override void InsertItem(int index, T item) => ExecuteOnSyncContext(() => base.InsertItem(index, item));
    protected override void RemoveItem(int index) => ExecuteOnSyncContext(() => base.RemoveItem(index));
    protected override void SetItem(int index, T item) => ExecuteOnSyncContext(() => base.SetItem(index, item));
    protected override void MoveItem(int oldIndex, int newIndex) => ExecuteOnSyncContext(() => base.MoveItem(oldIndex, newIndex));
    protected override void ClearItems() => ExecuteOnSyncContext(() => base.ClearItems());
}

And this AsyncRelayCommand class that I've made with help on StackOverflow (Russian Community). It doesn't freeze anything.

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

public class AsyncRelayCommand : IAsyncCommand
{
    private bool _isExecuting;
    private readonly SynchronizationContext _context;
    private readonly Action<object> _execute;
    private readonly Predicate<object> _canExecute;

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

    public AsyncRelayCommand(Action<object> execute, Predicate<object> canExecute = null) 
        => (_execute, _canExecute, _context) = (execute, canExecute, SynchronizationContext.Current);

    private void InvalidateRequerySuggested()
    {
        if (_context.Equals(SynchronizationContext.Current))
            CommandManager.InvalidateRequerySuggested();
        else
            _context.Send(_ => CommandManager.InvalidateRequerySuggested(), null);
    }

    public bool CanExecute(object parameter) => !_isExecuting && (_canExecute == null || _canExecute(parameter));

    public async Task ExecuteAsync(object parameter)
    {
        if (CanExecute(parameter))
        {
            try
            {
                _isExecuting = true;
                InvalidateRequerySuggested();
                await Task.Run(() => _execute(parameter));
            }
            finally
            {
                _isExecuting = false;
                InvalidateRequerySuggested();
            }
        }
    }

    public void Execute(object parameter) => _ = ExecuteAsync(parameter);
}

Usage as with regular RelayCommand class from this article.

private IAsyncCommand _myAsyncCommand;

// "lazy" instantiation with single instance
public IAsyncCommand MyAsyncCommand => _myAsyncCommand ?? (_myAsyncCommand = new AsyncRelayCommand(parameter =>
{

}));

<Button Content="Click me!" Command="{Binding MyAsyncCommand}" />

CommandParameter is also supported.

With this you don't need pushing collecton change calls into UI Thread, use it as in usual sync code. And Thread.Sleep() or heavy job in the command will not freeze UI because it will run on a separate thread.

Usage with your code

private IAsyncCommand _appearingCommand;
public AsyncObservableCollection<CarViewModel> cars; // are you sure that it must be public?

public HomeDetailPageViewModel(INavigationService navigationService)
        :base(navigationService)
{
    this.cars = new AsyncObservableCollection<CarViewModel>();
}

public AsyncObservableCollection<CarViewModel> Cars
{
    get => this.cars;
}

public IAsyncCommand AppearingCommand => _appearingCommand ?? (_appearingCommand = new AsyncRelayCommand(parameter =>
{
    // edit: fixed regarding to the accepted answer
    List<Car> carsFromEndpoint = CarEndpoint.GetAll(client.Context).ToList();
    foreach (var carFromEndpoint in carsFromEndpoint)
        this.Cars.Add(new CarViewModel
        {
            Manufacturer = carFromEndpoint.Code,
            Model = carFromEndpoint.Name,
            Price = carFromEndpoint.Price,
            Year = carFromEndpoint.Year
        });
}));

Answer 2

I know it is not a good idea to use Task.Run to make fake asynchronous calls

Using Task.Run to unblock a UI thread - even in a fake-asynchronous way - is fine.

I cannot change the endpoint of web service.

This sentence doesn't make as much sense. All web services are asynchronous by nature. However, it's possible that your client-side library is strictly synchronous, in which case Task.Run is a fine way to unblock the UI while calling it. But you'll be working around a limitation in the client-side library, not the web service itself.

I have a problem with display data in my list view.

Your problem is likely due to the IEnumerable<T>, not Task.Run. Here's some code I expect would work; the key is to move the ToList inside the Task.Run delegate:

public async Task OnAppearingCommandExecuted()
{
  using (UserDialogs.Instance.Loading("Loading..."))
  {
    this.Cars.Clear();
    List<Car> carsFromEndpoint = await Task.Run(() => CarEndpoint.GetAll(client.Context).ToList());

    foreach (var carFromEndpoint in carsFromEndpoint)
      this.Cars.Add(new CarViewModel
      {
        Manufacturer = carFromEndpoint.Code,
        Model = carFromEndpoint.Name,
        Price = carFromEndpoint.Price,
        Year = carFromEndpoint.Year
      });
  }
}

Notes:

• BeginInvokeOnMainThread is unnecessary if you remove the ConfigureAwait(false); the await resumes on the UI thread for us. In fact, BeginInvokeOnMainThread is a code smell.
• You probably don't want an asynchronous command here; you just want to asynchronously load data.