Remove INotifyPropertyChanged #1061
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implementing INotifyPropertyChanged seems not to be helpful currently. The interface was implemented since the very beginning of GirCore but the PropertyChanged event was never raised. As there were no complaints there is probably no need for this interface. Implementing it would mean that every object has 2 different ways of notifying about property changes (1. Notify event, 2. PropertyChanged) which seems a bit odd in the first place. As the notify event is the native system it should be preferred. While considering a possible implementation it came to mind that the PropertyChanged event should contain the managed name of the property. This would be the only sensible way for a C# dev, so the instance could potentially be compatible to other frameworks which require an instance of INotifyPropertyChanged and access the property via reflection. To raise the PropertyChanged event every object would always be required to subscribe to the Notify signal and forward it to PropertyChanged (after some property name mapping happend, which could get non trivial, too). If a user does need the INotifyPropertyChanged interface it could be implemented manually as a subclass and the mapping could be implemented manually.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #718