From 75225378d01fa33c63c7508286625174563f3f93 Mon Sep 17 00:00:00 2001 From: Roland Pheasant Date: Tue, 5 Oct 2021 12:49:45 +0100 Subject: [PATCH] List filter fix (#514) * Fix auto refresh / dynamic filter issue for observable list. Fixes #393. * Optimise joins by using ChangeAwareCache directly * Removed some of the fundamentalist fxcop rules as I have a different opinion and I am fed up with fx cop telling me what makes code more readable. --- src/DynamicData.Tests/AutoRefreshFilter.cs | 113 ++++++++++- src/DynamicData/Cache/Internal/FullJoin.cs | 148 +++++++------- src/DynamicData/Cache/Internal/InnerJoin.cs | 142 +++++++------ src/DynamicData/Cache/Internal/LeftJoin.cs | 190 +++++++++--------- .../Cache/Internal/LockFreeObservableCache.cs | 28 ++- src/DynamicData/Cache/Internal/RightJoin.cs | 128 ++++++------ src/DynamicData/List/Internal/Filter.cs | 159 +++++++-------- src/DynamicData/List/Internal/Transformer.cs | 22 +- src/analyzers.ruleset | 2 +- 9 files changed, 511 insertions(+), 421 deletions(-) diff --git a/src/DynamicData.Tests/AutoRefreshFilter.cs b/src/DynamicData.Tests/AutoRefreshFilter.cs index ae14237ff..e6d9576e2 100644 --- a/src/DynamicData.Tests/AutoRefreshFilter.cs +++ b/src/DynamicData.Tests/AutoRefreshFilter.cs @@ -1,6 +1,9 @@ using System; +using System.Collections.ObjectModel; using System.ComponentModel; - +using System.Reactive.Linq; +using System.Reactive.Subjects; +using DynamicData.Binding; using FluentAssertions; using Xunit; @@ -33,6 +36,97 @@ public void Test() obsListDerived.Count.Should().Be(3); obsListDerived.Items.Should().BeEquivalentTo(a0, i2, i3); } + + [Fact] + public void AutoRefreshWithObservablePredicate1() + { + var item1 = new ActivableItem + { + Activated = false + }; + + var items = new SourceList(); + items.Add(item1); + + var filterSubject = new BehaviorSubject>(_ => false); + + var obsListDerived = items + .Connect() + .AutoRefresh(i => i.Activated) + .Filter(filterSubject) + .Do(x => Console.WriteLine("Changes" + x.TotalChanges)) + .AsObservableList(); + + // Default filter predicate denies all items + // The binding collection should stay empty, until the predicate changes + obsListDerived.Count.Should().Be(0); + + item1.Activated = true; + obsListDerived.Count.Should().Be(0); + + item1.Activated = false; + obsListDerived.Count.Should().Be(0); + + item1.Activated = true; + obsListDerived.Count.Should().Be(0); + + // Changing predicate, all "Activated" items should added to the binding collection + filterSubject.OnNext(i => i.Activated); + + obsListDerived.Count.Should().Be(1); + obsListDerived.Items.Should().BeEquivalentTo(new[] { item1 }); + + // Changing property value + item1.Activated = false; + obsListDerived.Count.Should().Be(0); + } + + [Fact] + public void AutoRefreshWithObservablePredicate2() + { + var item1 = new ActivableItem + { + Activated = false + }; + + var items = new ObservableCollection(); + items.Add(item1); + + var filterSubject = new BehaviorSubject>(_ => false); + + var obsListDerived = items + .ToObservableChangeSet() + .AutoRefresh(i => i.Activated) + .Filter(filterSubject) + .AsObservableList(); + + // Default filter predicate denies all items + // The binding collection should stay empty, until the predicate changes + obsListDerived.Count.Should().Be(0); + + item1.Activated = true; + obsListDerived.Count.Should().Be(0); + + item1.Activated = false; + obsListDerived.Count.Should().Be(0); + + item1.Activated = true; + obsListDerived.Count.Should().Be(0); + + // Changing predicate, all "Activated" items should added to the binding collection + filterSubject.OnNext(i => i.Activated); + + obsListDerived.Count.Should().Be(1); + obsListDerived.Items.Should().BeEquivalentTo(new[] { item1 }); + + // Changing property value multiple times + item1.Activated = false; + item1.Activated = true; + item1.Activated = false; + item1.Activated = true; + obsListDerived.Count.Should().Be(1); + obsListDerived.Items.Should().BeEquivalentTo(new[] { item1 }); + } } public class Item : INotifyPropertyChanged @@ -59,4 +153,21 @@ public string Name } } } + + public class ActivableItem : INotifyPropertyChanged + { + private bool _activated; + + public bool Activated + { + get => _activated; + set + { + _activated = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Activated))); + } + } + + public event PropertyChangedEventHandler? PropertyChanged; + } } \ No newline at end of file diff --git a/src/DynamicData/Cache/Internal/FullJoin.cs b/src/DynamicData/Cache/Internal/FullJoin.cs index 8d7b029f4..114d0cd8e 100644 --- a/src/DynamicData/Cache/Internal/FullJoin.cs +++ b/src/DynamicData/Cache/Internal/FullJoin.cs @@ -42,97 +42,91 @@ public IObservable> Run() var rightCache = _right.Synchronize(locker).ChangeKey(_rightKeySelector).AsObservableCache(false); // joined is the final cache - var joinedCache = new LockFreeObservableCache(); + var joinedCache = new ChangeAwareCache(); - var leftLoader = leftCache.Connect().Subscribe( - changes => + var leftLoader = leftCache.Connect().Select(changes => { - joinedCache.Edit( - innerCache => - { - foreach (var change in changes.ToConcreteType()) - { - var left = change.Current; - var right = rightCache.Lookup(change.Key); + foreach (var change in changes.ToConcreteType()) + { + var left = change.Current; + var right = rightCache.Lookup(change.Key); - switch (change.Reason) - { - case ChangeReason.Add: - case ChangeReason.Update: - innerCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key); - break; - - case ChangeReason.Remove: - - if (!right.HasValue) - { - // remove from result because there is no left and no rights - innerCache.Remove(change.Key); - } - else - { - // update with no left value - innerCache.AddOrUpdate(_resultSelector(change.Key, Optional.None, right), change.Key); - } - - break; - - case ChangeReason.Refresh: - // propagate upstream - innerCache.Refresh(change.Key); - break; - } + switch (change.Reason) + { + case ChangeReason.Add: + case ChangeReason.Update: + joinedCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key); + break; + + case ChangeReason.Remove: + + if (!right.HasValue) + { + // remove from result because there is no left and no rights + joinedCache.Remove(change.Key); + } + else + { + // update with no left value + joinedCache.AddOrUpdate(_resultSelector(change.Key, Optional.None, right), change.Key); } - }); + + break; + + case ChangeReason.Refresh: + // propagate upstream + joinedCache.Refresh(change.Key); + break; + } + } + + return joinedCache.CaptureChanges(); }); - var rightLoader = rightCache.Connect().Subscribe( - changes => + var rightLoader = rightCache.Connect().Select(changes => { - joinedCache.Edit( - innerCache => - { - foreach (var change in changes.ToConcreteType()) + foreach (var change in changes.ToConcreteType()) + { + var right = change.Current; + var left = leftCache.Lookup(change.Key); + + switch (change.Reason) + { + case ChangeReason.Add: + case ChangeReason.Update: { - var right = change.Current; - var left = leftCache.Lookup(change.Key); + joinedCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key); + } + + break; - switch (change.Reason) + case ChangeReason.Remove: + { + if (!left.HasValue) { - case ChangeReason.Add: - case ChangeReason.Update: - { - innerCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key); - } - - break; - - case ChangeReason.Remove: - { - if (!left.HasValue) - { - // remove from result because there is no left and no rights - innerCache.Remove(change.Key); - } - else - { - // update with no right value - innerCache.AddOrUpdate(_resultSelector(change.Key, left, Optional.None), change.Key); - } - } - - break; - - case ChangeReason.Refresh: - // propagate upstream - innerCache.Refresh(change.Key); - break; + // remove from result because there is no left and no rights + joinedCache.Remove(change.Key); + } + else + { + // update with no right value + joinedCache.AddOrUpdate(_resultSelector(change.Key, left, Optional.None), change.Key); } } - }); + + break; + + case ChangeReason.Refresh: + // propagate upstream + joinedCache.Refresh(change.Key); + break; + } + } + + return joinedCache.CaptureChanges(); }); - return new CompositeDisposable(joinedCache.Connect().NotEmpty().SubscribeSafe(observer), leftCache, rightCache, leftLoader, joinedCache, rightLoader); + return new CompositeDisposable(leftLoader.Merge(rightLoader).SubscribeSafe(observer), leftCache, rightCache); }); } } diff --git a/src/DynamicData/Cache/Internal/InnerJoin.cs b/src/DynamicData/Cache/Internal/InnerJoin.cs index 1a141f70f..b5e721b6d 100644 --- a/src/DynamicData/Cache/Internal/InnerJoin.cs +++ b/src/DynamicData/Cache/Internal/InnerJoin.cs @@ -41,93 +41,89 @@ public InnerJoin(IObservable> left, IObservable(); - var leftLoader = leftCache.Connect().Subscribe( - changes => + var joinedCache = new ChangeAwareCache(); + + var leftLoader = leftCache.Connect().Select(changes => { - joinedCache.Edit( - innerCache => + foreach (var change in changes.ToConcreteType()) + { + TLeft left = change.Current; + var right = rightGrouped.Lookup(change.Key); + + if (right.HasValue) { - foreach (var change in changes.ToConcreteType()) + switch (change.Reason) { - TLeft left = change.Current; - var right = rightGrouped.Lookup(change.Key); + case ChangeReason.Add: + case ChangeReason.Update: + foreach (var keyvalue in right.Value.KeyValues) + { + joinedCache.AddOrUpdate(_resultSelector((change.Key, keyvalue.Key), left, keyvalue.Value), (change.Key, keyvalue.Key)); + } - if (right.HasValue) - { - switch (change.Reason) + break; + + case ChangeReason.Remove: + foreach (var keyvalue in right.Value.KeyValues) { - case ChangeReason.Add: - case ChangeReason.Update: - foreach (var keyvalue in right.Value.KeyValues) - { - innerCache.AddOrUpdate(_resultSelector((change.Key, keyvalue.Key), left, keyvalue.Value), (change.Key, keyvalue.Key)); - } - - break; - - case ChangeReason.Remove: - foreach (var keyvalue in right.Value.KeyValues) - { - innerCache.Remove((change.Key, keyvalue.Key)); - } - - break; - - case ChangeReason.Refresh: - foreach (var key in right.Value.Keys) - { - innerCache.Refresh((change.Key, key)); - } - - break; + joinedCache.Remove((change.Key, keyvalue.Key)); } - } + + break; + + case ChangeReason.Refresh: + foreach (var key in right.Value.Keys) + { + joinedCache.Refresh((change.Key, key)); + } + + break; } - }); + } + } + + return joinedCache.CaptureChanges(); }); - var rightLoader = rightCache.Connect().Subscribe( - changes => + var rightLoader = rightCache.Connect().Select(changes => { - joinedCache.Edit( - innerCache => + foreach (var change in changes.ToConcreteType()) + { + var leftKey = _rightKeySelector(change.Current); + switch (change.Reason) { - foreach (var change in changes.ToConcreteType()) - { - var leftKey = _rightKeySelector(change.Current); - switch (change.Reason) + case ChangeReason.Add: + case ChangeReason.Update: + // Update with right (and right if it is presents) + var right = change.Current; + var left = leftCache.Lookup(leftKey); + if (left.HasValue) { - case ChangeReason.Add: - case ChangeReason.Update: - // Update with right (and right if it is presents) - var right = change.Current; - var left = leftCache.Lookup(leftKey); - if (left.HasValue) - { - innerCache.AddOrUpdate(_resultSelector((leftKey, change.Key), left.Value, right), (leftKey, change.Key)); - } - else - { - innerCache.Remove((leftKey, change.Key)); - } - - break; - - case ChangeReason.Remove: - // remove from result because a right value is expected - innerCache.Remove((leftKey, change.Key)); - break; - - case ChangeReason.Refresh: - // propagate upstream - innerCache.Refresh((leftKey, change.Key)); - break; + joinedCache.AddOrUpdate(_resultSelector((leftKey, change.Key), left.Value, right), (leftKey, change.Key)); } - } - }); + else + { + joinedCache.Remove((leftKey, change.Key)); + } + + break; + + case ChangeReason.Remove: + // remove from result because a right value is expected + joinedCache.Remove((leftKey, change.Key)); + break; + + case ChangeReason.Refresh: + // propagate upstream + joinedCache.Refresh((leftKey, change.Key)); + break; + } + } + + return joinedCache.CaptureChanges(); }); - return new CompositeDisposable(joinedCache.Connect().NotEmpty().SubscribeSafe(observer), leftCache, rightCache, leftLoader, rightLoader, joinedCache); + + return new CompositeDisposable(leftLoader.Merge(rightLoader).SubscribeSafe(observer), leftCache, rightCache); }); } } diff --git a/src/DynamicData/Cache/Internal/LeftJoin.cs b/src/DynamicData/Cache/Internal/LeftJoin.cs index bea94fad1..45307826a 100644 --- a/src/DynamicData/Cache/Internal/LeftJoin.cs +++ b/src/DynamicData/Cache/Internal/LeftJoin.cs @@ -34,105 +34,101 @@ public IObservable> Run() { return Observable.Create>( observer => - { - var locker = new object(); - - // create local backing stores - var leftCache = _left.Synchronize(locker).AsObservableCache(false); - var rightCache = _right.Synchronize(locker).ChangeKey(_rightKeySelector).AsObservableCache(false); - - // joined is the final cache - var joinedCache = new LockFreeObservableCache(); - - var leftLoader = leftCache.Connect().Subscribe( - changes => + { + var locker = new object(); + + // create local backing stores + var leftCache = _left.Synchronize(locker).AsObservableCache(false); + var rightCache = _right.Synchronize(locker).ChangeKey(_rightKeySelector).AsObservableCache(false); + + // joined is the final cache + var joined = new ChangeAwareCache(); + + var leftLoader = leftCache.Connect().Select( + changes => + { + foreach (var change in changes.ToConcreteType()) + { + switch (change.Reason) { - joinedCache.Edit( - innerCache => - { - foreach (var change in changes.ToConcreteType()) - { - switch (change.Reason) - { - case ChangeReason.Add: - case ChangeReason.Update: - // Update with left (and right if it is presents) - var left = change.Current; - var right = rightCache.Lookup(change.Key); - innerCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key); - break; - - case ChangeReason.Remove: - // remove from result because a left value is expected - innerCache.Remove(change.Key); - break; - - case ChangeReason.Refresh: - // propagate upstream - innerCache.Refresh(change.Key); - break; - } - } - }); - }); - - var rightLoader = rightCache.Connect().Subscribe( - changes => + case ChangeReason.Add: + case ChangeReason.Update: + // Update with left (and right if it is presents) + var left = change.Current; + var right = rightCache.Lookup(change.Key); + joined.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key); + break; + + case ChangeReason.Remove: + // remove from result because a left value is expected + joined.Remove(change.Key); + break; + + case ChangeReason.Refresh: + // propagate upstream + joined.Refresh(change.Key); + break; + } + } + + return joined.CaptureChanges(); + }); + + var rightLoader = rightCache.Connect().Select( + changes => + { + foreach (var change in changes.ToConcreteType()) + { + var right = change.Current; + var left = leftCache.Lookup(change.Key); + + switch (change.Reason) { - joinedCache.Edit( - innerCache => - { - foreach (var change in changes.ToConcreteType()) - { - var right = change.Current; - var left = leftCache.Lookup(change.Key); - - switch (change.Reason) - { - case ChangeReason.Add: - case ChangeReason.Update: - { - if (left.HasValue) - { - // Update with left and right value - innerCache.AddOrUpdate(_resultSelector(change.Key, left.Value, right), change.Key); - } - else - { - // remove if it is already in the cache - innerCache.Remove(change.Key); - } - } - - break; - - case ChangeReason.Remove: - { - if (left.HasValue) - { - // Update with no right value - innerCache.AddOrUpdate(_resultSelector(change.Key, left.Value, Optional.None), change.Key); - } - else - { - // remove if it is already in the cache - innerCache.Remove(change.Key); - } - } - - break; - - case ChangeReason.Refresh: - // propagate upstream - innerCache.Refresh(change.Key); - break; - } - } - }); - }); - - return new CompositeDisposable(joinedCache.Connect().NotEmpty().SubscribeSafe(observer), leftCache, rightCache, leftLoader, joinedCache, rightLoader); - }); + case ChangeReason.Add: + case ChangeReason.Update: + { + if (left.HasValue) + { + // Update with left and right value + joined.AddOrUpdate(_resultSelector(change.Key, left.Value, right), change.Key); + } + else + { + // remove if it is already in the cache + joined.Remove(change.Key); + } + } + + break; + + case ChangeReason.Remove: + { + if (left.HasValue) + { + // Update with no right value + joined.AddOrUpdate(_resultSelector(change.Key, left.Value, Optional.None), change.Key); + } + else + { + // remove if it is already in the cache + joined.Remove(change.Key); + } + } + + break; + + case ChangeReason.Refresh: + // propagate upstream + joined.Refresh(change.Key); + break; + } + } + + return joined.CaptureChanges(); + }); + + return new CompositeDisposable(leftLoader.Merge(rightLoader).SubscribeSafe(observer), leftCache, rightCache); + }); } } } \ No newline at end of file diff --git a/src/DynamicData/Cache/Internal/LockFreeObservableCache.cs b/src/DynamicData/Cache/Internal/LockFreeObservableCache.cs index 738206106..de2a37e15 100644 --- a/src/DynamicData/Cache/Internal/LockFreeObservableCache.cs +++ b/src/DynamicData/Cache/Internal/LockFreeObservableCache.cs @@ -22,13 +22,16 @@ namespace DynamicData.Cache.Internal public sealed class LockFreeObservableCache : IObservableCache where TKey : notnull { - private readonly ISubject> _changes = new Subject>(); + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "Disposed with _cleanUp")] + private readonly Subject> _changes = new Subject>(); - private readonly ISubject> _changesPreview = new Subject>(); + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "Disposed with _cleanUp")] + private readonly Subject> _changesPreview = new Subject>(); - private readonly IDisposable _cleanUp; + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "Disposed with _cleanUp")] + private readonly Subject _countChanged = new Subject(); - private readonly ISubject _countChanged = new Subject(); + private readonly IDisposable _cleanUp; private readonly ChangeAwareCache _innerCache = new(); @@ -89,16 +92,25 @@ public LockFreeObservableCache() /// public IEnumerable> KeyValues => _innerCache.KeyValues; - /// ns>An observable that emits the change set. + /// public IObservable> Connect(Func? predicate = null, bool suppressEmptyChangeSets = true) { return Observable.Defer( () => { - var initial = _innerCache.GetInitialUpdates(predicate); - var changes = Observable.Return(initial).Concat(_changes); + var initial = InternalEx.Return(() => _innerCache.GetInitialUpdates(predicate)); + var changes = initial.Concat(_changes); + + if (predicate != null) + { + changes = changes.Filter(predicate, suppressEmptyChangeSets); + } + else if (suppressEmptyChangeSets) + { + changes = changes.NotEmpty(); + } - return (predicate is null ? changes : changes.Filter(predicate)).NotEmpty(); + return changes; }); } diff --git a/src/DynamicData/Cache/Internal/RightJoin.cs b/src/DynamicData/Cache/Internal/RightJoin.cs index bee654e0d..5b29e2385 100644 --- a/src/DynamicData/Cache/Internal/RightJoin.cs +++ b/src/DynamicData/Cache/Internal/RightJoin.cs @@ -43,87 +43,81 @@ public IObservable> Run() var rightGrouped = _right.Synchronize(locker).GroupWithImmutableState(_rightKeySelector).AsObservableCache(false); // joined is the final cache - var joinedCache = new LockFreeObservableCache(); + var joinedCache = new ChangeAwareCache(); - var rightLoader = rightCache.Connect().Subscribe( - changes => + var rightLoader = rightCache.Connect().Select(changes => { - joinedCache.Edit( - innerCache => + foreach (var change in changes.ToConcreteType()) + { + var leftKey = _rightKeySelector(change.Current); + switch (change.Reason) { - foreach (var change in changes.ToConcreteType()) - { - var leftKey = _rightKeySelector(change.Current); - switch (change.Reason) - { - case ChangeReason.Add: - case ChangeReason.Update: - // Update with right (and right if it is presents) - var right = change.Current; - var left = leftCache.Lookup(leftKey); - innerCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key); - break; - - case ChangeReason.Remove: - // remove from result because a right value is expected - innerCache.Remove(change.Key); - break; - - case ChangeReason.Refresh: - // propagate upstream - innerCache.Refresh(change.Key); - break; - } - } - }); + case ChangeReason.Add: + case ChangeReason.Update: + // Update with right (and right if it is presents) + var right = change.Current; + var left = leftCache.Lookup(leftKey); + joinedCache.AddOrUpdate(_resultSelector(change.Key, left, right), change.Key); + break; + + case ChangeReason.Remove: + // remove from result because a right value is expected + joinedCache.Remove(change.Key); + break; + + case ChangeReason.Refresh: + // propagate upstream + joinedCache.Refresh(change.Key); + break; + } + } + + return joinedCache.CaptureChanges(); }); - var leftLoader = leftCache.Connect().Subscribe( - changes => + var leftLoader = leftCache.Connect().Select(changes => { - joinedCache.Edit( - innerCache => + foreach (var change in changes.ToConcreteType()) + { + TLeft left = change.Current; + var right = rightGrouped.Lookup(change.Key); + + if (right.HasValue) { - foreach (var change in changes.ToConcreteType()) + switch (change.Reason) { - TLeft left = change.Current; - var right = rightGrouped.Lookup(change.Key); + case ChangeReason.Add: + case ChangeReason.Update: + foreach (var keyvalue in right.Value.KeyValues) + { + joinedCache.AddOrUpdate(_resultSelector(keyvalue.Key, left, keyvalue.Value), keyvalue.Key); + } + + break; - if (right.HasValue) - { - switch (change.Reason) + case ChangeReason.Remove: + foreach (var keyvalue in right.Value.KeyValues) { - case ChangeReason.Add: - case ChangeReason.Update: - foreach (var keyvalue in right.Value.KeyValues) - { - innerCache.AddOrUpdate(_resultSelector(keyvalue.Key, left, keyvalue.Value), keyvalue.Key); - } - - break; - - case ChangeReason.Remove: - foreach (var keyvalue in right.Value.KeyValues) - { - innerCache.AddOrUpdate(_resultSelector(keyvalue.Key, Optional.None, keyvalue.Value), keyvalue.Key); - } - - break; - - case ChangeReason.Refresh: - foreach (var key in right.Value.Keys) - { - innerCache.Refresh(key); - } - - break; + joinedCache.AddOrUpdate(_resultSelector(keyvalue.Key, Optional.None, keyvalue.Value), keyvalue.Key); } - } + + break; + + case ChangeReason.Refresh: + foreach (var key in right.Value.Keys) + { + joinedCache.Refresh(key); + } + + break; } - }); + } + } + + return joinedCache.CaptureChanges(); }); - return new CompositeDisposable(joinedCache.Connect().NotEmpty().SubscribeSafe(observer), leftCache, rightCache, rightLoader, joinedCache, leftLoader); + return new CompositeDisposable(leftLoader.Merge(rightLoader).SubscribeSafe(observer), leftCache, rightCache); }); } } diff --git a/src/DynamicData/List/Internal/Filter.cs b/src/DynamicData/List/Internal/Filter.cs index 2298cbf43..95e37a105 100644 --- a/src/DynamicData/List/Internal/Filter.cs +++ b/src/DynamicData/List/Internal/Filter.cs @@ -39,64 +39,60 @@ public IObservable> Run() { return Observable.Create>( observer => - { - var locker = new object(); + { + var locker = new object(); - Func predicate = _ => false; - var all = new List(); - var filtered = new ChangeAwareList(); - var immutableFilter = _predicate is not null; + Func predicate = _ => false; + var all = new List(); + var filtered = new ChangeAwareList(); + var immutableFilter = _predicate is not null; - IObservable> predicateChanged; + IObservable> predicateChanged; - if (immutableFilter) - { - predicateChanged = Observable.Never>(); - predicate = _predicate ?? predicate; - } - else - { - if (_predicates is null) - { - throw new InvalidOperationException("The predicates is not set and the change is not a immutableFilter."); - } + if (immutableFilter) + { + predicateChanged = Observable.Never>(); + predicate = _predicate ?? predicate; + } + else + { + if (_predicates is null) + throw new InvalidOperationException("The predicates is not set and the change is not a immutableFilter."); - predicateChanged = _predicates.Synchronize(locker).Select( - newPredicate => - { - predicate = newPredicate; - return Requery(predicate, all, filtered); - }); - } + predicateChanged = _predicates.Synchronize(locker).Select( + newPredicate => + { + predicate = newPredicate; + return Requery(predicate, all, filtered); + }); + } + + /* + * Apply the transform operator so 'IsMatch' state can be evaluated and captured one time only + * This is to eliminate the need to re-apply the predicate when determining whether an item was previously matched, + * which is essential when we have mutable state + */ + + // Need to get item by index and store it in the transform + var filteredResult = _source.Synchronize(locker).Transform((t, previous) => + { + var wasMatch = previous.ConvertOr(p => p!.IsMatch, () => false); + return new ItemWithMatch(t, predicate(t), wasMatch); + }, + true) + .Select(changes => + { + // keep track of all changes if filtering on an observable + if (!immutableFilter) + all.Clone(changes); - /* - * Apply the transform operator so 'IsMatch' state can be evaluated and captured one time only - * This is to eliminate the need to re-apply the predicate when determining whether an item was previously matched, - * which is essential when we have mutable state - */ + return Process(filtered, changes); + }); - // Need to get item by index and store it in the transform - var filteredResult = _source.Synchronize(locker).Transform( - (t, previous) => - { - var wasMatch = previous.ConvertOr(p => p.IsMatch, () => false); - return new ItemWithMatch(t, predicate(t), wasMatch); - }, - true).Select( - changes => - { - // keep track of all changes if filtering on an observable - if (!immutableFilter) - { - all.Clone(changes); - } - - return Process(filtered, changes); - }); - - return predicateChanged.Merge(filteredResult).NotEmpty().Select(changes => changes.Transform(iwm => iwm.Item)) // use convert, not transform - .SubscribeSafe(observer); - }); + return predicateChanged.Merge(filteredResult).NotEmpty() + .Select(changes => changes.Transform(iwm => iwm.Item)) // use convert, not transform + .SubscribeSafe(observer); + }); } private static IChangeSet Process(ChangeAwareList filtered, IChangeSet changes) @@ -110,9 +106,7 @@ private static IChangeSet Process(ChangeAwareList { var change = item.Item; if (change.Current.IsMatch) - { filtered.Add(change.Current); - } break; } @@ -147,9 +141,7 @@ private static IChangeSet Process(ChangeAwareList else { if (wasMatch) - { filtered.Remove(change.Previous.Value); - } } break; @@ -177,9 +169,7 @@ private static IChangeSet Process(ChangeAwareList else { if (wasMatch) - { filtered.Remove(change.Current); - } } break; @@ -237,7 +227,10 @@ private IChangeSet Requery(Func predicate, List Requery(Func predicate, List + private class ItemWithMatch : IEquatable { + public T Item { get; } + + public bool IsMatch { get; set; } + + public bool WasMatch { get; set; } + public ItemWithMatch(T item, bool isMatch, bool wasMatch = false) - : this() { Item = item; IsMatch = isMatch; WasMatch = wasMatch; } - public T Item { get; } - - public bool IsMatch { get; } - - public bool WasMatch { get; } - - /// Returns a value that indicates whether the values of two objects are equal. - /// The first value to compare. - /// The second value to compare. - /// true if the and parameters have the same value; otherwise, false. - public static bool operator ==(ItemWithMatch left, ItemWithMatch right) + public bool Equals(ItemWithMatch? other) { - return Equals(left, right); + if (ReferenceEquals(null, other)) return false; + if (ReferenceEquals(this, other)) return true; + return EqualityComparer.Default.Equals(Item, other.Item); } - /// Returns a value that indicates whether two objects have different values. - /// The first value to compare. - /// The second value to compare. - /// true if and are not equal; otherwise, false. - public static bool operator !=(ItemWithMatch left, ItemWithMatch right) + public override bool Equals(object? obj) { - return !Equals(left, right); + if (ReferenceEquals(null, obj)) return false; + if (ReferenceEquals(this, obj)) return true; + if (obj.GetType() != GetType()) return false; + return Equals((ItemWithMatch)obj); } - public bool Equals(ItemWithMatch other) + public override int GetHashCode() { - return EqualityComparer.Default.Equals(Item, other.Item); + return EqualityComparer.Default.GetHashCode(Item!); } - public override bool Equals(object? obj) + public static bool operator ==(ItemWithMatch? left, ItemWithMatch? right) { - return obj is ItemWithMatch value && Equals(value); + return Equals(left, right); } - public override int GetHashCode() + public static bool operator !=(ItemWithMatch? left, ItemWithMatch? right) { - return Item is null ? 0 : EqualityComparer.Default.GetHashCode(Item); + return !Equals(left, right); } public override string ToString() => $"{Item}, (was {IsMatch} is {WasMatch}"; diff --git a/src/DynamicData/List/Internal/Transformer.cs b/src/DynamicData/List/Internal/Transformer.cs index f6ef97085..f74bda243 100644 --- a/src/DynamicData/List/Internal/Transformer.cs +++ b/src/DynamicData/List/Internal/Transformer.cs @@ -33,18 +33,16 @@ public Transformer(IObservable> source, Func> Run() { - return _source.Scan( - new ChangeAwareList(), - (state, changes) => - { - Transform(state, changes); - return state; - }).Select( - transformed => - { - var changed = transformed.CaptureChanges(); - return changed.Transform(container => container.Destination); - }); + return _source.Scan(new ChangeAwareList(), (state, changes) => + { + Transform(state, changes); + return state; + }) + .Select(transformed => + { + var changed = transformed.CaptureChanges(); + return changed.Transform(container => container.Destination); + }); } private void Transform(ChangeAwareList transformed, IChangeSet changes) diff --git a/src/analyzers.ruleset b/src/analyzers.ruleset index 1594432d7..b0ebedb97 100644 --- a/src/analyzers.ruleset +++ b/src/analyzers.ruleset @@ -169,7 +169,7 @@ - +