Skip to content

Making internal & private classes sealed to avoid virtual dispatch, plus some Dictionary capacity tweaks#3552

Merged
iancooper merged 1 commit into
BrighterCommand:masterfrom
Henr1k80:master
Mar 5, 2025
Merged

Making internal & private classes sealed to avoid virtual dispatch, plus some Dictionary capacity tweaks#3552
iancooper merged 1 commit into
BrighterCommand:masterfrom
Henr1k80:master

Conversation

@Henr1k80

@Henr1k80 Henr1k80 commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

As the good Stephen Toub explains, there is good reasons to make classes sealed.

I can also make it a warning, if all new code does not obey this, by enabling CA1852
Then I would also have to mark all tests & examples sealed, no problem with me, but a LOT of diff lines.
Alternatively make a config for tests that explicitly ignore the rule.

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@Henr1k80

Henr1k80 commented Mar 5, 2025

Copy link
Copy Markdown
Contributor Author

A MSSQL unit test is failing, can it be a timing issue?
That dispatched Now does match query Now?

@iancooper

Copy link
Copy Markdown
Member

Hi @Henr1k80. It is a good point. I guess there is no real muscle memory for us around making something sealed. Probably in case we feel the need to extend it later.

@iancooper

Copy link
Copy Markdown
Member

A MSSQL unit test is failing, can it be a timing issue?
As we push towards V10, a few fragile tests will occasionally fail. We will probably pick them up if they keep failing and see if we can rewrite, or just mark them as fragile so they don't run in CI.

But expect a little bit of instability as we push toward our next RC.

@iancooper iancooper merged commit a2d3d7b into BrighterCommand:master Mar 5, 2025
@iancooper iancooper added 3 - Done v10 .NET Pull requests that update .net code Performance Improvement labels Mar 5, 2025
@iancooper

Copy link
Copy Markdown
Member

I can also make it a warning, if all new code does not obey this, by enabling CA1852

We should probably do this, as I think we will unlikely remember without a prompt for a while. Do you want to do that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Done .NET Pull requests that update .net code Performance Improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants