Skip to content

Commit fd1d7b5

Browse files
RolandPheasantRolandPheasant
authored
Revert to original behaviour of unsorted bound list in Bind() operator which uses remove / add instead of replace + add an option to use replace. Fixes #641. Fixes #640. Reverts #381 (#642)
1 parent aaa9896 commit fd1d7b5

7 files changed

Lines changed: 194 additions & 52 deletions

File tree

‎src/DynamicData.Tests/Binding/ObservableCollectionBindCacheFixture.cs‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,35 @@ public void RemoveSourceRemovesFromTheDestination()
7676
[Fact]
7777
public void UpdateToSourceSendsReplaceOnDestination()
7878
{
79-
var person = new Person("Adult1", 50);
80-
var anotherPerson = new Person("Adult1", 51);
81-
NotifyCollectionChangedAction action = default;
82-
_source.AddOrUpdate(person);
79+
RunTest(true);
80+
RunTest(false);
81+
8382

84-
using (_collection.ObserveCollectionChanges().Select(x => x.EventArgs.Action).Subscribe(updateType => action = updateType))
83+
void RunTest(bool useReplace)
8584
{
86-
_source.AddOrUpdate(anotherPerson);
87-
}
85+
var collection = new ObservableCollectionExtended<Person>();
86+
87+
using var source = new SourceCache<Person, string>(p => p.Name);
88+
using var binder = source.Connect().Bind(collection, useReplaceForUpdates: useReplace).Subscribe();
8889

89-
action.Should().Be(NotifyCollectionChangedAction.Replace, "The notification type should be Replace");
90+
91+
NotifyCollectionChangedAction action = default;
92+
source.AddOrUpdate(new Person("Adult1", 50));
93+
94+
using (collection.ObserveCollectionChanges().Select(x => x.EventArgs.Action).Subscribe(updateType => action = updateType))
95+
{
96+
source.AddOrUpdate(new Person("Adult1", 51));
97+
}
98+
99+
if (useReplace)
100+
{
101+
action.Should().Be(NotifyCollectionChangedAction.Replace);
102+
}
103+
else
104+
{
105+
action.Should().Be(NotifyCollectionChangedAction.Add);
106+
}
107+
}
90108
}
91109

92110
[Fact]

‎src/DynamicData.Tests/Binding/ReadonlyCollectionBindCacheFixture.cs‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
using System;
2+
using System.Collections.ObjectModel;
3+
using System.Collections.Specialized;
4+
using System.Linq;
5+
using System.Reactive.Linq;
6+
using DynamicData.Binding;
7+
using DynamicData.Tests.Domain;
8+
using FluentAssertions;
9+
using Xunit;
10+
11+
namespace DynamicData.Tests.Binding;
12+
13+
public class ReadonlyCollectionBindCacheFixture : IDisposable
14+
{
15+
private readonly IDisposable _binder;
16+
private readonly ReadOnlyObservableCollection<Person> _collection;
17+
private readonly RandomPersonGenerator _generator = new();
18+
private readonly ISourceCache<Person, string> _source;
19+
20+
public ReadonlyCollectionBindCacheFixture()
21+
{
22+
_source = new SourceCache<Person, string>(p => p.Name);
23+
_binder = _source.Connect().Bind(out _collection).Subscribe();
24+
}
25+
26+
[Fact]
27+
public void AddToSourceAddsToDestination()
28+
{
29+
var person = new Person("Adult1", 50);
30+
_source.AddOrUpdate(person);
31+
32+
_collection.Count.Should().Be(1, "Should be 1 item in the collection");
33+
_collection.First().Should().Be(person, "Should be same person");
34+
}
35+
36+
[Fact]
37+
public void BatchAdd()
38+
{
39+
var people = _generator.Take(100).ToList();
40+
_source.AddOrUpdate(people);
41+
42+
_collection.Count.Should().Be(100, "Should be 100 items in the collection");
43+
_collection.Should().BeEquivalentTo(_collection, "Collections should be equivalent");
44+
}
45+
46+
[Fact]
47+
public void BatchRemove()
48+
{
49+
var people = _generator.Take(100).ToList();
50+
_source.AddOrUpdate(people);
51+
_source.Clear();
52+
_collection.Count.Should().Be(0, "Should be 100 items in the collection");
53+
}
54+
55+
public void Dispose()
56+
{
57+
_binder.Dispose();
58+
_source.Dispose();
59+
}
60+
61+
[Fact]
62+
public void RemoveSourceRemovesFromTheDestination()
63+
{
64+
var person = new Person("Adult1", 50);
65+
_source.AddOrUpdate(person);
66+
_source.Remove(person);
67+
68+
_collection.Count.Should().Be(0, "Should be 1 item in the collection");
69+
}
70+
71+
[Fact]
72+
public void UpdateToSourceSendsReplaceOnDestination()
73+
{
74+
RunTest(true);
75+
RunTest(false);
76+
77+
78+
void RunTest(bool useReplace)
79+
{
80+
using var source = new SourceCache<Person, string>(p => p.Name);
81+
using var binder = source.Connect().Bind(out var collection, useReplaceForUpdates: useReplace).Subscribe();
82+
83+
NotifyCollectionChangedAction action = default;
84+
source.AddOrUpdate(new Person("Adult1", 50));
85+
86+
using (collection.ObserveCollectionChanges().Select(x => x.EventArgs.Action).Subscribe(updateType => action = updateType))
87+
{
88+
source.AddOrUpdate(new Person("Adult1", 51));
89+
}
90+
91+
if (useReplace)
92+
{
93+
action.Should().Be(NotifyCollectionChangedAction.Replace);
94+
}
95+
else
96+
{
97+
action.Should().Be(NotifyCollectionChangedAction.Add);
98+
}
99+
}
100+
}
101+
102+
[Fact]
103+
public void UpdateToSourceUpdatesTheDestination()
104+
{
105+
var person = new Person("Adult1", 50);
106+
var personUpdated = new Person("Adult1", 51);
107+
_source.AddOrUpdate(person);
108+
_source.AddOrUpdate(personUpdated);
109+
110+
_collection.Count.Should().Be(1, "Should be 1 item in the collection");
111+
_collection.First().Should().Be(personUpdated, "Should be updated person");
112+
}
113+
}

‎src/DynamicData/Binding/ObservableCollectionAdaptor.cs‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Diagnostics.CodeAnalysis;
77

88
using DynamicData.Cache.Internal;
9+
using DynamicData.Kernel;
910

1011
namespace DynamicData.Binding;
1112

@@ -72,16 +73,19 @@ public class ObservableCollectionAdaptor<TObject, TKey> : IObservableCollectionA
7273
private readonly Cache<TObject, TKey> _cache = new();
7374

7475
private readonly int _refreshThreshold;
76+
private readonly bool _useReplaceForUpdates;
7577

7678
private bool _loaded;
7779

7880
/// <summary>
7981
/// Initializes a new instance of the <see cref="ObservableCollectionAdaptor{TObject, TKey}"/> class.
8082
/// </summary>
8183
/// <param name="refreshThreshold">The threshold before a reset notification is triggered.</param>
82-
public ObservableCollectionAdaptor(int refreshThreshold = 25)
84+
/// <param name="useReplaceForUpdates"> Use replace instead of remove / add for updates. </param>
85+
public ObservableCollectionAdaptor(int refreshThreshold = 25, bool useReplaceForUpdates = false)
8386
{
8487
_refreshThreshold = refreshThreshold;
88+
_useReplaceForUpdates = useReplaceForUpdates;
8589
}
8690

8791
/// <summary>
@@ -120,24 +124,51 @@ public void Adapt(IChangeSet<TObject, TKey> changes, IObservableCollection<TObje
120124
}
121125
}
122126

123-
private static void DoUpdate(IChangeSet<TObject, TKey> updates, IObservableCollection<TObject> list)
127+
private void DoUpdate(IChangeSet<TObject, TKey> changes, IObservableCollection<TObject> list)
124128
{
125-
foreach (var update in updates)
129+
void Amend(Change<TObject, TKey> change)
126130
{
127-
switch (update.Reason)
131+
switch (change.Reason)
128132
{
129133
case ChangeReason.Add:
130-
list.Add(update.Current);
134+
list.Add(change.Current);
131135
break;
132136

133137
case ChangeReason.Remove:
134-
list.Remove(update.Current);
138+
list.Remove(change.Current);
135139
break;
136140

137141
case ChangeReason.Update:
138-
list.Replace(update.Previous.Value, update.Current);
142+
143+
// Remove / Add is default as some platforms do not support list[index] = XXX notifications.
144+
if (_useReplaceForUpdates)
145+
{
146+
list.Replace(change.Previous.Value, change.Current);
147+
}
148+
else
149+
{
150+
list.Remove(change.Previous.Value);
151+
list.Add(change.Current);
152+
}
153+
139154
break;
140155
}
141156
}
157+
158+
if (changes is IList<Change<TObject, TKey>> iList)
159+
{
160+
// allocation free enumeration
161+
foreach (var change in EnumerableIList.Create(iList))
162+
{
163+
Amend(change);
164+
}
165+
}
166+
else
167+
{
168+
foreach (var update in changes)
169+
{
170+
Amend(update);
171+
}
172+
}
142173
}
143174
}

‎src/DynamicData/Cache/ObservableCacheEx.cs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,11 @@ public static IObservable<IChangeSet<TObject, TKey>> BatchIf<TObject, TKey>(this
585585
/// <typeparam name="TKey">The type of the key.</typeparam>
586586
/// <param name="source">The source.</param>
587587
/// <param name="destination">The destination.</param>
588+
/// <param name="refreshThreshold">The number of changes before a reset notification is triggered.</param>
589+
/// <param name="useReplaceForUpdates"> Use replace instead of remove / add for updates. NB: Some platforms to not support replace notifications for binding.</param>
588590
/// <returns>An observable which will emit change sets.</returns>
589591
/// <exception cref="System.ArgumentNullException">source.</exception>
590-
public static IObservable<IChangeSet<TObject, TKey>> Bind<TObject, TKey>(this IObservable<IChangeSet<TObject, TKey>> source, IObservableCollection<TObject> destination)
592+
public static IObservable<IChangeSet<TObject, TKey>> Bind<TObject, TKey>(this IObservable<IChangeSet<TObject, TKey>> source, IObservableCollection<TObject> destination, int refreshThreshold = 25, bool useReplaceForUpdates = false)
591593
where TKey : notnull
592594
{
593595
if (source is null)
@@ -600,7 +602,7 @@ public static IObservable<IChangeSet<TObject, TKey>> Bind<TObject, TKey>(this IO
600602
throw new ArgumentNullException(nameof(destination));
601603
}
602604

603-
var updater = new ObservableCollectionAdaptor<TObject, TKey>();
605+
var updater = new ObservableCollectionAdaptor<TObject, TKey>(refreshThreshold, useReplaceForUpdates);
604606
return source.Bind(destination, updater);
605607
}
606608

@@ -745,11 +747,12 @@ public static IObservable<IChangeSet<TObject, TKey>> Bind<TObject, TKey>(this IO
745747
/// <typeparam name="TKey">The type of the key.</typeparam>
746748
/// <param name="source">The source.</param>
747749
/// <param name="readOnlyObservableCollection">The resulting read only observable collection.</param>
748-
/// <param name="resetThreshold">The number of changes before a reset event is called on the observable collection.</param>
750+
/// <param name="resetThreshold">The number of changes before a reset notification is triggered.</param>
751+
/// <param name="useReplaceForUpdates"> Use replace instead of remove / add for updates. NB: Some platforms to not support replace notifications for binding.</param>
749752
/// <param name="adaptor">Specify an adaptor to change the algorithm to update the target collection.</param>
750753
/// <returns>An observable which will emit change sets.</returns>
751754
/// <exception cref="System.ArgumentNullException">source.</exception>
752-
public static IObservable<IChangeSet<TObject, TKey>> Bind<TObject, TKey>(this IObservable<IChangeSet<TObject, TKey>> source, out ReadOnlyObservableCollection<TObject> readOnlyObservableCollection, int resetThreshold = 25, IObservableCollectionAdaptor<TObject, TKey>? adaptor = null)
755+
public static IObservable<IChangeSet<TObject, TKey>> Bind<TObject, TKey>(this IObservable<IChangeSet<TObject, TKey>> source, out ReadOnlyObservableCollection<TObject> readOnlyObservableCollection, int resetThreshold = 25, bool useReplaceForUpdates = false, IObservableCollectionAdaptor<TObject, TKey>? adaptor = null)
753756
where TKey : notnull
754757
{
755758
if (source is null)
@@ -759,7 +762,7 @@ public static IObservable<IChangeSet<TObject, TKey>> Bind<TObject, TKey>(this IO
759762

760763
var target = new ObservableCollectionExtended<TObject>();
761764
var result = new ReadOnlyObservableCollection<TObject>(target);
762-
var updater = adaptor ?? new ObservableCollectionAdaptor<TObject, TKey>(resetThreshold);
765+
var updater = adaptor ?? new ObservableCollectionAdaptor<TObject, TKey>(resetThreshold, useReplaceForUpdates);
763766
readOnlyObservableCollection = result;
764767
return source.Bind(target, updater);
765768
}

‎src/DynamicData/List/Change.cs‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,8 @@
22
// Roland Pheasant licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for full license information.
44

5-
using System;
6-
using System.Collections.Generic;
7-
85
using DynamicData.Kernel;
96

10-
#pragma warning disable 1591
11-
127
// ReSharper disable once CheckNamespace
138
namespace DynamicData;
149

@@ -103,12 +98,12 @@ public Change(ListChangeReason reason, T current, Optional<T> previous, int curr
10398

10499
if (reason == ListChangeReason.Replace && !previous.HasValue)
105100
{
106-
throw new ArgumentException("For ChangeReason.Change, must supply previous value");
101+
throw new ArgumentException("For ChangeReason.Replace, must supply previous value");
107102
}
108103

109104
if (reason == ListChangeReason.Refresh && currentIndex < 0)
110105
{
111-
throw new ArgumentException("For ChangeReason.Refresh, must supply and index");
106+
throw new ArgumentException("For ChangeReason.Refresh, must supply ad index");
112107
}
113108

114109
Reason = reason;
@@ -193,8 +188,5 @@ public override int GetHashCode()
193188
}
194189

195190
/// <inheritdoc />
196-
public override string ToString()
197-
{
198-
return $"{Reason}. {Range.Count} changes";
199-
}
191+
public override string ToString() => $"{Reason}. {Range.Count} changes";
200192
}

‎src/DynamicData/List/RangeChange.cs‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -60,35 +60,23 @@ private RangeChange()
6060
/// Adds the specified item to the range.
6161
/// </summary>
6262
/// <param name="item">The item.</param>
63-
public void Add(T item)
64-
{
65-
_items.Add(item);
66-
}
63+
public void Add(T item) => _items.Add(item);
6764

6865
/// <inheritdoc/>
69-
public IEnumerator<T> GetEnumerator()
70-
{
71-
return _items.GetEnumerator();
72-
}
66+
public IEnumerator<T> GetEnumerator() => _items.GetEnumerator();
7367

7468
/// <summary>
7569
/// Inserts the item in the range at the specified index.
7670
/// </summary>
7771
/// <param name="index">The index.</param>
7872
/// <param name="item">The item.</param>
79-
public void Insert(int index, T item)
80-
{
81-
_items.Insert(index, item);
82-
}
73+
public void Insert(int index, T item) => _items.Insert(index, item);
8374

8475
/// <summary>
8576
/// Sets the index of the starting index of the range.
8677
/// </summary>
8778
/// <param name="index">The index.</param>
88-
public void SetStartingIndex(int index)
89-
{
90-
Index = index;
91-
}
79+
public void SetStartingIndex(int index) => Index = index;
9280

9381
/// <summary>
9482
/// Returns a <see cref="string" /> that represents this instance.
@@ -99,8 +87,5 @@ public void SetStartingIndex(int index)
9987
public override string ToString() => $"Range<{typeof(T).Name}>. Count={Count}";
10088

10189
/// <inheritdoc/>
102-
IEnumerator IEnumerable.GetEnumerator()
103-
{
104-
return GetEnumerator();
105-
}
90+
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
10691
}

‎version.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "7.10",
2+
"version": "7.11",
33
"publicReleaseRefSpec": [
44
"^refs/heads/main$", // we release out of master
55
"^refs/heads/preview/.*", // we release previews

0 commit comments

Comments
 (0)