ListBox async filtering, cancelling previous task on filter update

Question

I'm trying to come up with a nice way of filtering a ListBox asynchronously, to keep the UI responsive. The filtering gets fired off asynchronously each time the filter text is changed. This needs to cancel all previous filtering operations, so the updates to the filtered list don't conflict.

It seems that a list of CancellationTokenSource objects are required, so that each task can be cancelled independently.

Below is the best that I've managed to come up with so far. It seems to me that there must be a nicer way of doing it.

ViewModel - MainPresenter.cs

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;

namespace AsyncListBoxFilter.ViewModels
{
    public sealed class MainPresenter : INotifyPropertyChanged
    {
        private const int ITEM_COUNT = 1000;
        private static readonly Random Rand = new Random();

        private readonly ICollection<CancellationTokenSource> _cancellationTokenSources
            = new List<CancellationTokenSource>();

        private readonly ICollection<string> _allItems;
        private string _filterString;

        public event PropertyChangedEventHandler PropertyChanged;

        public MainPresenter()
        {
            _allItems = new List<string>(
                Enumerable.Range(0, ITEM_COUNT)
                    .Select(_ => CreateRandomString()));
            FilteredItems = new ObservableCollection<string>(_allItems);
        }

        public ObservableCollection<string> FilteredItems { get; private set; }

        public string FilterString
        {
            get => _filterString;
            set
            {
                _filterString = value;
                OnPropertyChanged();
                foreach (var source in _cancellationTokenSources)
                    source.Cancel();
                Task.Run(FilterAsync);
            }
        }

        private async Task FilterAsync()
        {
            var source = new CancellationTokenSource();
            _cancellationTokenSources.Add(source);
            var cancellationToken = source.Token;

            var filterString = FilterString; // Take a copy in case it changes
            var filtered = new ObservableCollection<string>();
            await Task.Run(() =>
            {
                var token = cancellationToken;

                foreach (var item in _allItems)
                {
                    if (token.IsCancellationRequested)
                        break;

                    Thread.Sleep(1); // Slow down for testing

                    if (item.Contains(filterString ?? string.Empty))
                        filtered.Add(item);
                }
            }, cancellationToken);

            if (cancellationToken.IsCancellationRequested)
            {
                Console.WriteLine($"'{filterString}' Cancelled");
            }
            else
            {
                FilteredItems = filtered;
                OnPropertyChanged(nameof(FilteredItems));
                Console.WriteLine($"'{filterString}' Success");
            }
        }

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

        private static string CreateRandomString()
        {
            char CreateRandomChar() => (char) (65 + Rand.Next(26));
            return new string(Enumerable.Range(0, Rand.Next(3, 10))
                .Select(_ => CreateRandomChar()).ToArray());
        }
    }
}

View - MainWindow.xaml

<Window x:Class="AsyncListBoxFilter.Views.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:ViewModels="clr-namespace:AsyncListBoxFilter.ViewModels"
        Title="Async ListBox Filter"
        Height="600" Width="800"
        WindowStartupLocation="CenterScreen">

    <Window.DataContext>
        <ViewModels:MainPresenter />
    </Window.DataContext>

    <DockPanel>
        <TextBox DockPanel.Dock="Top"
                 Text="{Binding FilterString, UpdateSourceTrigger=PropertyChanged}"
                 Margin="5" />
        <ListBox ItemsSource="{Binding FilteredItems}" Margin="5" />
    </DockPanel>

</Window>

Answer 1

This solution does not convince me because...

---

public string FilterString
{
    get => _filterString;
    set
    {
        _filterString = value;
        OnPropertyChanged();
        foreach (var source in _cancellationTokenSources)
            source.Cancel();
        Task.Run(FilterAsync);
    }
}

Don't do so much work in a setter. This is not the right place for that. You should implement it as a command like FilterCommand and fire it when the text changes.

---

The CancellationTokenSource must be disposed. You are not doing that yet.

---

You don't use Thread.Sleep when working with the async/await pattern because you don't want to put the entire thread to sleep. What you want is Task.Delay.

---

var token = cancellationToken;

What purpose does this line have?

---

FilteredItems = filtered;
OnPropertyChanged(nameof(FilteredItems));

The FilterAsync should not be triggering the property-changed event. The assignment should be all it does. On the other hand if its type is an ObservableCollection then you should use this for updates or make it an ordinary list becuase it's confusing when you have an observable that you are not using as such.

---

ICollection<CancellationTokenSource> _cancellationTokenSources

You never remove any items from this collection and you also never dispose them.

Answer 2

You might want to consider that _filterString and value might be the same, in which case do you want the rest of the setter logic to run? There may be a lot work for very little benefit.

if(_filterString.Equals(value)) return;
OnPropertyChanged();
...

Answer 3

One more point I want to add is that, Call filter after some duration when the user is entering the input and not for every text change. Setter should be simple and move the logic out of setter.