4
\$\begingroup\$

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>
\$\endgroup\$
3
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Thank you for taking the time to reply. I don't think I spent enough time putting together my question, as some side issues have distracted from my core interest of the best pattern for handling tasks overriding each other in this context. I'll have a think and update the question to be clearer. \$\endgroup\$ – Scroog1 Apr 12 '18 at 7:45
  • \$\begingroup\$ @Scroog1 ok, but remember to not modify the code anymore... it's not allowed after receiving answers. If you intend to do it... it's better to ask a new question. \$\endgroup\$ – t3chb0t Apr 12 '18 at 7:47
  • \$\begingroup\$ A fresh question might be a better approach. Thanks. \$\endgroup\$ – Scroog1 Apr 12 '18 at 8:00
1
\$\begingroup\$

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();
...
\$\endgroup\$
  • \$\begingroup\$ Is there any particular reason you would use _filterString.Equals(value) and not _filterString == value in this case? \$\endgroup\$ – Scroog1 Apr 12 '18 at 7:54
0
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ What's the benefit of the delay? And what is a sensible delay time in this context? \$\endgroup\$ – Scroog1 Apr 13 '18 at 6:41
  • \$\begingroup\$ filtering and cancellation need not to be done, if the user is going to type still. A sensible delay would be around 200 to 400 ms. \$\endgroup\$ – vishnu vardhan Apr 13 '18 at 8:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.