🔒 Mark classes sealed that can be sealed #40
Merged
Conversation
This file contains hidden or 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
tomrijnbeek
added
the
breaking change
paulcscharf
approved these changes
Apr 26, 2022
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.
✨ What's this?
This PR marks all the classes that were never designed to be inherited from as sealed. It also does some minor clean-up around structs as well.
🔗 Relationships
Closes #31
🔍 Why do we want this?
Either design for inheritance, or prohibit it. These classes were not designed to become base classes. If we truly care to have consumers implement their own - say - root control, then we should use an interface or specifically designed abstract base class instead.
🏗 How is it done?
Clicked through all the files. Where a class could be marked as sealed, I did.
💥 Breaking changes
Any dependency implementing subclasses of the now sealed classes will break. This is why I intend to increase the minor version next build. We're still in pre-release, so we can't change the major version.
🔬 Why not another way?
There's only one way to mark classes sealed 😜
🦋 Side effects
There are now some eventargs classes not sealed (because they're in turn inherited from), while others are. Perhaps they should all be sealed and use composition, or we should have only abstract classes for inheritance and sealed classes to actually use, or we don't care enough.
💡 Review hints
🤷♂️