Cysharp/ObservableCollections

ObservableDictionary doesn't seem to work well in WPF

Closed this issue · 3 comments

I clone the repo and edit the wpf test like below:

// ObservableCollections\sandbox\WpfApp\MainWindow.xaml.cs
// ...
public class ViewModel
{
    private ObservableDictionary<int, int> observableList { get; } = [];

    public INotifyCollectionChangedSynchronizedView<int> ItemsView { get; }

    public ReactiveCommand<Unit> ClearCommand { get; } = new ReactiveCommand<Unit>();

    public ReactiveCommand<Unit> RemoveCommand { get; } = new ReactiveCommand<Unit>();

    public ViewModel()
    {
        observableList.Add(1, 1);
        observableList.Add(2, 2);

        ItemsView = observableList.CreateView(x => x.Value).ToNotifyCollectionChanged();

        BindingOperations.EnableCollectionSynchronization(ItemsView, new object());

        // var iii = 10;
        ClearCommand.Subscribe(_ =>
        {
            // observableList.Add(iii++);
            observableList.Clear();
        });

        RemoveCommand.Subscribe(_ =>
        {
            observableList.Remove(1);
        });
    }
}
// ...
// ObservableCollections\sandbox\WpfApp\MainWindow.xaml
// ...
<StackPanel>
    <ListBox ItemsSource="{Binding ItemsView}"/>
    <Button Content="Clear" Command="{Binding ClearCommand}" />
    <Button Content="Remove 1" Command="{Binding RemoveCommand}" />
</StackPanel>
// ...

Run the application, and click the remove button. It thows InvalidOperation with message Collection Remove event must specify item position.
Stack trace below:

at MS.Internal.Data.EnumerableCollectionView.ProcessCollectionChanged(NotifyCollectionChangedEventArgs args)
at System.Windows.Data.CollectionView.PostChange(NotifyCollectionChangedEventArgs args)
at ObservableCollections.Internal.NotifyCollectionChangedSynchronizedView2.OnCollectionChanged(SynchronizedViewChangedEventArgs2& args) in D:\Documents\ObservableCollections\src\ObservableCollections\Internal\NotifyCollectionChangedSynchronizedView.cs:line 72
at ObservableCollections.Internal.NotifyCollectionChangedSynchronizedView2.ObservableCollections.ISynchronizedViewFilter<T,TView>.OnCollectionChanged(SynchronizedViewChangedEventArgs2& eventArgs)
at ObservableCollections.SynchronizedViewFilterExtensions.InvokeOnRemove[T,TView](ISynchronizedViewFilter2 filter, T value, TView view, Int32 oldIndex) in D:\Documents\ObservableCollections\src\ObservableCollections\ISynchronizedViewFilter.cs:line 103 at ObservableCollections.ObservableDictionary2.View1.SourceCollectionChanged(NotifyCollectionChangedEventArgs1& e) in D:\Documents\ObservableCollections\src\ObservableCollections\ObservableDictionary.Views.cs:line 145
at ObservableCollections.ObservableDictionary2.Remove(TKey key) in D:\Documents\ObservableCollections\src\ObservableCollections\ObservableDictionary.cs:line 176 at WpfApp.ViewModel.<.ctor>b__12_2(Unit _) in D:\Documents\ObservableCollections\sandbox\WpfApp\MainWindow.xaml.cs:line 100 at R3.AnonymousObserver1.OnNextCore(T value)

I might understand why the exception was thrown, a possible solution would be add a backed collection (for example a sorted list) for binding in my code.

I did some research, and found that this is a decades-old pain point in regards to ListView, ListBox, etc: you pass an IEnumerable to bind to, but what they really want is IList, because they expect an integer index to synchronize things.

Dictionaries do not store data in a deterministic order, so ObservableDictionary can't guarantee a specific integer index for each key-value-pair, which is why it returns -1 for oldStartingIndex in its NotifyCollectionChangedEventArgs for remove. This causes the error you're experiencing. Incidentally, ObservableHashSet will have the same behavior.

Perhaps ObservableCollections may benefit from an ObservableKeyedCollection (which can be indexed both as a dictionary and a list).

I did some research, and found that this is a decades-old pain point in regards to ListView, ListBox, etc: you pass an IEnumerable to bind to, but what they really want is IList, because they expect an integer index to synchronize things.

Dictionaries do not store data in a deterministic order, so ObservableDictionary can't guarantee a specific integer index for each key-value-pair, which is why it returns -1 for oldStartingIndex in its NotifyCollectionChangedEventArgs for remove. This causes the error you're experiencing. Incidentally, ObservableHashSet will have the same behavior.

Perhaps ObservableCollections may benefit from an ObservableKeyedCollection (which can be indexed both as a dictionary and a list).

I also see the post when I encounter the problem and search for solution.
For now I write my own list collection for dictionary to support list view binding.