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

sealed some internal classes in Controls.Core that are not inherited from (1) #17669

Merged
merged 4 commits into from
Nov 12, 2023
Merged

sealed some internal classes in Controls.Core that are not inherited from (1) #17669

merged 4 commits into from
Nov 12, 2023

Conversation

Lehonti
Copy link
Contributor

@Lehonti Lehonti commented Sep 26, 2023

Description of Change

Sealed some classes. They are not part of the public API, but this change can help us better track code invariants

@Lehonti Lehonti requested a review from a team as a code owner September 26, 2023 21:15
@ghost ghost added the community ✨ Community Contribution label Sep 26, 2023
@ghost
Copy link

ghost commented Sep 26, 2023

Hey there @Lehonti! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@hartez
Copy link
Contributor

hartez commented Sep 26, 2023

What problem are you trying to solve with this?

@Lehonti
Copy link
Contributor Author

Lehonti commented Sep 26, 2023

What problem are you trying to solve with this?

There is no problem as such. I am just trying to make the code easier to read and understand, and with less pitfalls (if a programmer sees that a class is unsealed, they may think that it's prepared to handle scenarios that it actually has not been planned for).

@trivalik
Copy link
Contributor

trivalik commented Oct 2, 2023

Additionally I heard of this: https://www.meziantou.net/performance-benefits-of-sealed-class.htm

@Eilon Eilon added the area-architecture Issues with code structure, SDK structure, implementation details label Oct 17, 2023
@jfversluis
Copy link
Member

@jonathanpeppers is this something we should consider from a perf standpoint?

@jonathanpeppers
Copy link
Member

It would help a very small amount, but if these are internal there is not really a reason not to do it?

@jfversluis jfversluis enabled auto-merge (squash) November 12, 2023 09:19
@jfversluis
Copy link
Member

/azp run MAUI-UITests-public

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfversluis jfversluis merged commit c4479e6 into dotnet:main Nov 12, 2023
@Lehonti Lehonti deleted the improvement1 branch November 12, 2023 14:42
@github-actions github-actions bot locked and limited conversation to collaborators Dec 13, 2023
@samhouts samhouts added the fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-architecture Issues with code structure, SDK structure, implementation details community ✨ Community Contribution fixed-in-8.0.6 Look for this fix in 8.0.6 SR1!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants