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

Improve the code readability of health info method #1393

Closed
wants to merge 1 commit into from

Conversation

jokoyoski
Copy link

In the refactored code:

The calculation of the total variable has been moved within the if condition to eliminate the need for a separate variable declaration.
Instead of comparing total with 0 in the if statement, we directly compare successes + failures with 0.
The calculation of failureRate has been separated into its own variable for clarity.
The result is returned as a HealthInfo object, with the successes + failures value and the failureRate value passed as arguments.
These changes improve readability and maintain the original functionality of the code.

@jokoyoski
Copy link
Author

jokoyoski commented Jul 10, 2023 via email

@martincostello
Copy link
Member

Thanks for your suggestion, but I don't agree that this improves the readability in this case.

In the worst case this performs the same addition three times now (which is why total was used to store the computation) so degrades performance, and also changes the indenting to not match the existing level of indentation used in the code style.

@martintmk
Copy link
Contributor

@jokoyoski

Thanks for your contribution, although this change might not be merged, you can check #1290 which proposes similar cleanup in the Polly codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants