reactivemarbles/DynamicData

[Bug]: System.ArgumentException: index cannot be negative

IanRawley opened this issue ยท 3 comments

Describe the bug ๐Ÿž

I'm trying to transform the values in an AvaloniaDictionary (a dictionary implementing INotifyCollectionChanged) into a ReadOnlyObservableCollection for use in an application using the following:

_hotkeyMap.ToObservableChangeSet<AvaloniaDictionary<Domain, Char?>, KeyValuePair<Domain, Char?>>() .Transform((x) => x.Value) .ObserveOn(RxApp.MainThreadScheduler) .Bind(out _hotkeysInUse) .Subscribe();

Upon adding the first item to the dictionary an ArgumentException is being thrown, with the message "index cannot be negative", stack trace from exception as follows.
DynamicData.dll!DynamicData.ChangeAwareList<System.Collections.Generic.KeyValuePair<silmetra.plotsafe2.data.Model.Domain, char?>>.Insert(int index, System.Collections.Generic.KeyValuePair<silmetra.plotsafe2.data.Model.Domain, char?> item) Line 219 C#
DynamicData.dll!DynamicData.Binding.ObservableCollectionEx.ToObservableChangeSet.AnonymousMethod__1(DynamicData.ChangeAwareList<System.Collections.Generic.KeyValuePair<silmetra.plotsafe2.data.Model.Domain, char?>> list, System.Reactive.EventPattern<System.Collections.Specialized.NotifyCollectionChangedEventArgs> args) Line 161 C#
System.Reactive.dll!System.Reactive.Linq.ObservableImpl.Scan<System.Reactive.EventPattern<System.Collections.Specialized.NotifyCollectionChangedEventArgs>, DynamicData.ChangeAwareList<System.Collections.Generic.KeyValuePair<silmetra.plotsafe2.data.Model.Domain, char?>>>._.OnNext(System.Reactive.EventPattern<System.Collections.Specialized.NotifyCollectionChangedEventArgs> value) Line 40 C#
System.Reactive.dll!System.Reactive.Subjects.Subject<System.Reactive.EventPattern<System.Collections.Specialized.NotifyCollectionChangedEventArgs>>.OnNext(System.Reactive.EventPattern<System.Collections.Specialized.NotifyCollectionChangedEventArgs> value) Line 147 C#
Avalonia.Base.dll!Avalonia.Collections.AvaloniaDictionary<silmetra.plotsafe2.data.Model.Domain, char?>.NotifyAdd(silmetra.plotsafe2.data.Model.Domain key, char? value) Line 230 C#

Tracking it down, it seems to be coming from a similar place to #683 in ObservableCollectionEx

`case NotifyCollectionChangedAction.Add when changes.NewItems is not null:
{
if (changes.NewItems.Count == 1 && changes.NewItems[0] is T item)
{
list.Insert(changes.NewStartingIndex, item);
}
else
{
list.InsertRange(changes.NewItems.Cast(), changes.NewStartingIndex);
}

                                break;
                            }`

where NewStartingIndex is -1. The AvaloniaDictionary is doing the following:

if (CollectionChanged != null) { var e = new NotifyCollectionChangedEventArgs( NotifyCollectionChangedAction.Add, new[] { new KeyValuePair<TKey, TValue>(key, value) }, -1); CollectionChanged(this, e); }

which is where I believe the -1 is coming from, but that also seems to be valid according to the documentation for NotifyCollectionChangedEventArgs. Is this something I'm doing wrong, or is this a bug in ObservableCollectionEx as it appears to be?

Step to reproduce

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Reproduction repository

None as yet.

Expected behavior

No exceptions should be thrown.

Screenshots ๐Ÿ–ผ๏ธ

No response

IDE

Visual Studio 2022

Operating system

Windows

Version

No response

Device

No response

DynamicData Version

No response

Additional information โ„น๏ธ

No response

It seems to me the -1 index never occurs in other platforms but can do so with the AvaloniaDictionary. I suspect this could be fixed with an additional check on whether the index is -1. In which case use add rather than insert.

So, this would be the piece of DynamicData where the problem is?

public static IObservable<IChangeSet<T>> ToObservableChangeSet<TCollection, T>(this TCollection source)
    where TCollection : INotifyCollectionChanged, IEnumerable<T>
    where T : notnull

Yeah, my recollection is that an Index of -1 is entirely valid, within INotifyCollectionChanged so we should definitely be able to handle it. Doesn't mean we need to propagate that through our own systems.

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.