RolandPheasant / DynamicData.Snippets

'101 samples' of dynamic data and rx in action
139 stars 13 forks source link

`RemoveMany` from a merged `SourceList` causes `Clear` #10

Open Nintynuts opened 6 months ago

Nintynuts commented 6 months ago

I'm guessing it's due to some optimisation, but this test fails, and the console output shows a clear.


[TestClass]
public class DynamicDataIssues
{
    private interface ICommonInterface { }
    private struct A : ICommonInterface { }
    private class B : ICommonInterface { }

    [TestMethod]
    public void RemoveAllFromMerged()
    {
        A[] fixedItems = { new(), new() };
        SourceList<B> moreItems = new();

        var disposable = fixedItems.AsObservableChangeSet(completable: true).Transform(i => (ICommonInterface)i)
            .Merge(moreItems.Connect().Transform(i => (ICommonInterface)i))
            .Do(x => Console.WriteLine($"+{x.Adds}, -{x.Removes}, {x.First()}"))
            // The sorting is to keep things in the order I merged them, but I would prefer to not need to do this.
            .Sort(SortExpressionComparer<ICommonInterface>
                .Ascending(i => i is not A)
                .ThenByAscending(i => i is not B))
            .Bind(out var allItems)
            .Subscribe();

        Assert.AreEqual(2, allItems.Count);

        // These work as expected
        moreItems.Add(new B());
        moreItems.Add(new B());

        // But these result it the fixed items being removed
        moreItems.RemoveMany(moreItems.Items.ToArray());

        Assert.AreEqual(2, allItems.Count);

        Assert.AreEqual(2, allItems.OfType<A>().Count());
    }
}

I've worked around the issue by removing items gradially rather than all in one go, but if I do a .Clear() the whole merged collection is cleared, which is also wrong. Is there more correct way to do this?

Thanks

Update: seems likely to be coming from here:

https://github.com/reactivemarbles/DynamicData/blob/76fd915924fab0e6756038f50a4fff4464a4ed00/src/DynamicData/List/ChangeAwareList.cs#L139

Is there a better way to prevent this than to remove all items individually?

Nintynuts commented 5 months ago

If this is the wrong place for this, I'm sorry. I can create it somewhere else.

JakenVeina commented 5 months ago

Sorry, activity on this particular repo doesn't pop up in my feed, and probably the same for @dwcullop. And @RolandPheasant has been on vacation.

No need to move, now that it's here, but yeah, in the future, https://github.com/reactivemarbles/DynamicData is the best spot for submitting issues.

The problem is that you cannot use .Merge() on DynamicData streams, and it's for exactly this kind of reason. DynamicData collections are only "logical" collections, built up from a stream of changes made. When you create two separate logical collections, and then just merge all the changesets together, without any consideration for which of the two logical collections they belong to, you end up with a corrupted downstream collection. You need to use one of the DynamicData .MergeXXX() operators, which will maintain that awareness. I.E. when a changeset comes in that says to clear one of the two source collections, it will recognize that the downstream "merged" collection is logically built from more than just that one source, and translate the upstream changeset into a changeset that will correctly-preserve other items in the downstream collection.

At a glance, what you probably want is...

namespace DynamicData;

public static class ObservableListEx
{
    public static IObservable<IChangeSet<TObject>> MergeChangeSets<TObject>(
            this    IObservable<IChangeSet<TObject>>    source,
                    IObservable<IChangeSet<TObject>>    other,
                    IEqualityComparer<TObject>?         equalityComparer    = null,
                    IScheduler?                         scheduler           = null,
                    bool                                completable         = true)
        where TObject : notnull;
}
Nintynuts commented 5 months ago

Ah, great. I suspected there would be something like that.

I was trying to find relevant examples in the various places you have them, but couldn't see anything similar to what I was trying to do (I guess because I was doing the wrong thing). It's great that you have examples, but they're a bit fragmented and it seems like there are some holes. Also, it's not clear when should or shouldn't use something as in this case. When should I use Merge rather than the suffixed method you mentioned? Could it be an overload which does the right thing based on the input types?

Another example I got even less far with was trying to join two sets of different data together into view models. I found the Join method, but I had no idea what to do with the duration delegates. I expected to have a left and right key selector then to need to transform them into my VMs.

It seems like this system should be really powerful and flexible, but I don't understand it well enough yet and need a crash course.