Skip to content

Conversation

@drewnoakes
Copy link
Member

Avoid unnecessary HealthStatus comparison

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.
    • There's no open issue for this. It's a drive-by micro-optimisation.

Description

It's only necessary to check currentValue == HealthStatus.Unhealthy when currentValue changes. During the first iteration, it is known to be Healthy.

It's only necessary to check `currentValue == HealthStatus.Unhealthy` when `currentValue` changes. During the first iteration, it is known to be `Healthy`.
@ghost ghost added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Oct 1, 2024
return currentValue;
if (currentValue == HealthStatus.Unhealthy)
{
// Game over, man! Game over!
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it worth considering replacing this comment with something more appropriate?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think I'd encourage people to write comments in this tone/register, but it wasn't introduced by this change and doesn't seem specifically inappropriate, so I'm okay with leaving it for now.

@amcasey amcasey merged commit 3bc8d5b into main Oct 3, 2024
27 checks passed
@amcasey amcasey deleted the drnoakes/heath-status-comparison branch October 3, 2024 20:42
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Oct 3, 2024
captainsafia pushed a commit that referenced this pull request Dec 31, 2024
It's only necessary to check `currentValue == HealthStatus.Unhealthy` when `currentValue` changes. During the first iteration, it is known to be `Healthy`.
captainsafia pushed a commit that referenced this pull request Feb 11, 2025
It's only necessary to check `currentValue == HealthStatus.Unhealthy` when `currentValue` changes. During the first iteration, it is known to be `Healthy`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants