Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detecting ISet<> and IList<> member changes #3308

Closed
polymath74 opened this issue May 5, 2023 · 3 comments
Closed

Detecting ISet<> and IList<> member changes #3308

polymath74 opened this issue May 5, 2023 · 3 comments

Comments

@polymath74
Copy link

polymath74 commented May 5, 2023

What happened?

ISet<> and IList<> membership changes used to produce a PropertyChanged notification. This no longer happens (as of about v10.20 ish, maybe earlier). This happens with both local and sync.

  1. Was this intentional?

The documentation pages ( https://www.mongodb.com/docs/realm/sdk/dotnet/model-data/data-types/collections/ and https://www.mongodb.com/docs/realm/sdk/dotnet/model-data/data-types/sets/ ) say fields of these collection types implement INotifyCollectionChanged. The C# properties, declared as ISet<> and IList<>, do not actually include this interface. BUT fortunately, it does actually work, if the field is cast as INotifyCollectionChanged.

  1. Should the documentation or the listed interfaces be updated to indicate how to do this? (This would have saved me some time trying to work out what was going on and how to fix it.)

Repro steps

  1. Define a RealmObject with an ISet<string> or IList<string> field.
  2. Register a PropertyChanged event listener.
  3. Add a set member in a write transaction.
  4. See whether event listener is triggered.

Version

10.21.1

What Atlas Services are you using?

Atlas Device Sync

What type of application is this?

WPF

Client OS and version

Windows 10 22H2 22621.1555

Code snippets

RealmTest.zip

Stacktrace of the exception/crash you're getting

No response

Relevant log output

No response

@nirinchev
Copy link
Member

nirinchev commented May 8, 2023

I can't reproduce this. I've attached a trivial console app that adds and removes items from a list/set and I seem to be getting all property change notifications correctly.

Regarding the interfaces - you're right, it may be a little confusing, but when designing the API, we wanted to use the system interfaces as much as possible so your models don't take a dependency on Realm beyond inheriting from the base class/adding the marker interface for codegen. This means that our collection types are less expressive than they could be. The underlying type that we use for ISet/IList properties does implement INotifyPropertyChanged and INotifyCollectionChanged and since the primary use case has been to just pass those collections to a data binding engine (which attempts the cast), it has worked fairly well, even if a little magically.

SetPropertyChanged.zip

Edit: re-reading your comment, it may seem I misunderstood the problem you're describing. Is the issue that we previously used to raise PropertyChanged notifications on the parent object and now we no longer do? If that's the case, then yes, it's intentional as this behavior resulted in a pretty bad experience when using these collections in a data-binding scenario. Since any change to the collection would tell the binding engine that the property itself changed, it wasn't taking advantage of the granular notifications delivered by the CollectionChanged subscription, meaning the entire container was re-rendering every time.

I can see that there's a "Data Binding" section at the bottom of the Sets docs page, but a similar section is missing on the Collections page. Is your suggestion that we add it there or do you have other ideas for how we could make it more obvious that collections emit notifications, even if the system interfaces don't communicate that?

@sync-by-unito sync-by-unito bot added the Waiting-For-Reporter Waiting for more information from the reporter before we can proceed label May 8, 2023
@polymath74
Copy link
Author

Thanks @nirinchev yes you are correct on all counts. It would be helpful if there was an example in the docs showing how to subscribe to CollectionChanged events for a collection and/or set field, using an explicit cast.

@github-actions github-actions bot added Needs-Attention Reporter has responded. Review comment. and removed Waiting-For-Reporter Waiting for more information from the reporter before we can proceed labels May 12, 2023
@nirinchev
Copy link
Member

Gotcha and I agree - the docs could be improved. I filed a docs ticket to improve this and will close this one as working as designed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants