DynamicData icon indicating copy to clipboard operation
DynamicData copied to clipboard

[Bug]: FilterOnObservable().Transform().Bind() causes Exception

Open J0nnyI opened this issue 3 years ago • 6 comments

Version: Current (7.4.3)

I was able to write a minimal Unittest to reproduce the issue, shuffling the operators solves the problem but has performance implications.

Exception:

System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
   at DynamicData.List.Internal.Transformer`2.Transform(ChangeAwareList`1 transformed, IChangeSet`1 changes) in /_/src/DynamicData/List/Internal/Transformer.cs:line 50
   at DynamicData.List.Internal.Transformer`2.<Run>b__4_0(ChangeAwareList`1 state, IChangeSet`1 changes) in /_/src/DynamicData/List/Internal/Transformer.cs:line 38
   at System.Reactive.Linq.ObservableImpl.Scan`2._.OnNext(TSource value) in /_/Rx.NET/Source/src/System.Reactive/Linq/Observable/Scan.cs:line 40

Unittest for reproduction:

[Test]
public void Bind_Transform_and_FilterOnObservable()
{
    var count = 3;
    var list = new SourceList<string>();
    list.AddRange(Enumerable.Range(1, count).Select(c => $"item {c}"));
    var bindedList = new ObservableCollectionExtended<string>();
    list.Connect()
        .FilterOnObservable(_ => Observable.Return(true))
        .Transform(str => str)
        .Bind(bindedList)
        .Subscribe(
            _ => { },
            ex => Assert.Fail("exception thrown: {0}", ex) // causes the test to fail
        );
}

J0nnyI avatar Nov 22 '21 17:11 J0nnyI

Yikes, how did I miss this.

I just ran the reproduction and I can confirm that I too see the issue. I'll investigate.

RolandPheasant avatar Apr 29 '22 15:04 RolandPheasant

Out of interest, what are you trying to achieve? FilterOnObservable is designed to apply a filter based on an observed value on individual items in the collection.

There is an overload to Filter where the predicate can be changes via an observable - confusing I think.

RolandPheasant avatar Apr 29 '22 16:04 RolandPheasant

Thanks for working on the issue. I am working on a client-only application where the data is stored in SourceCaches. When loading the data into the ViewModel and preparing it for the view I

note: since the internal INotifyPropertyChanged trigger (filter on property etc) renders the app unusable (probably for Perforemance reasons? I had to update the filter by hand when each item fires a property changed event

I hope this helps in clarifying where I am coming from

J0nnyI avatar Apr 30 '22 00:04 J0nnyI

Interesting to see that you are no longer using FilterOnObservable().

I would certainly favour doing what you are doing in the current code and I am also very tempted to mark FilterOnObservable as obsolete as it is seriously problematic.

Regarding the perf problems of 'since the internal INotifyPropertyChanged trigger', can you clarify what you mean. Historically I have subscribed using AutoRefresh and WhenValueChanged to rapidly moving large (in the tens of thousands) record sets. The key, and it's an absolute must for performance on those operators is to provide a timespan for the buffer overloads.

RolandPheasant avatar May 04 '22 10:05 RolandPheasant

Also, OnAdded / On Removed could you just use AsObservableList(), or just bind?

Lot's of ways of doing so and there are similar examples here The Snippets Project

RolandPheasant avatar May 04 '22 10:05 RolandPheasant

I apologize for the late reply, for some reason github did not send ma an notification.

Regarding the perf problems of 'since the internal INotifyPropertyChanged trigger', can you clarify what you mean. Historically I have subscribed using AutoRefresh and WhenValueChanged to rapidly moving large (in the tens of thousands) record sets. The key, and it's an absolute must for performance on those operators is to provide a timespan for the buffer overloads.

Thanks for the TimeSpan suggestion, I'll take a look at it one I revisit the code. Regarding WhenValueChanged, I think the Cache/Observable list got a Method which should update all downstream operators once a value has changed. using this method creates a massive load, possibly even without updates, which causes the UI to drop to roughly 10 frames per Minute. (I verified this issue and tracked it down to this exact method call) Once I revisit the code I can provide you with he exact method and possible even a UnitTest to limit the scope of the problem, like I did before.

Also, OnAdded / On Removed could you just use AsObservableList(), or just bind?

I believe this is not possible in this case, since I have to consume these specific events to execute an action, which means even if I use AsObservableList(), I would have to get the add and delete event either way.

Lot's of ways of doing so and there are similar examples here The Snippets Project

I did not know about this project, does it also contain an example about Pagination? I was unable to find a wpf example of how to integrate al ListView with the Operator.

J0nnyI avatar May 30 '22 19:05 J0nnyI

It's only taken a year to fix this one. Better late than never.

RolandPheasant avatar Nov 24 '22 15:11 RolandPheasant

Thank you, has it actually been that difficult to fix or were there simply more pressing matters? I'm just interested

J0nnyI avatar Nov 24 '22 17:11 J0nnyI

I previously looked at the issue several times and scratched my head thinking WTF. However when I looked at it this time, the answer was obvious! Sometimes problem solving is like that.

A factor which helped me now is I feel engaged in the project again. It was always a labour of love but there have been some spans of time where I've had not spare mental capacity.

RolandPheasant avatar Nov 24 '22 22:11 RolandPheasant

I'm also keen to evolve TailBlazer again. 6.5 after I developed it!

RolandPheasant avatar Nov 24 '22 22:11 RolandPheasant

Hopefully it can be merged soon, but have to resolve a build issue (nothing to do with the code changes).

RolandPheasant avatar Nov 24 '22 22:11 RolandPheasant

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.

github-actions[bot] avatar Dec 10 '22 01:12 github-actions[bot]