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

General - Optimize some if statements #10302

Merged
merged 1 commit into from
Sep 11, 2024
Merged

General - Optimize some if statements #10302

merged 1 commit into from
Sep 11, 2024

Conversation

PabstMirror
Copy link
Contributor

a tiny bit faster to remove the not
these are mostly in functions that may be called more frequently

@PabstMirror PabstMirror added kind/optimization Release Notes: **IMPROVED:** impact/trivial labels Sep 9, 2024
@PabstMirror PabstMirror added this to the 3.18.0 milestone Sep 9, 2024
@LinkIsGrim
Copy link
Contributor

Code is fine, but I think this is less readable. Worth it?

@BrettMayson
Copy link
Member

imo it is more readable, with maybe some exception, when it is as readable. The code is the same, it's still if else, it's just swapped order

@rautamiekka
Copy link
Contributor

Way more readable cuz it ain't backwards.

Copy link
Contributor

@LinkIsGrim LinkIsGrim left a comment

Choose a reason for hiding this comment

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

the people have spoken

Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

Code is fine, but I think this is less readable.

I feel the same way for most isNull/isNil checks, but it doesn't matter much.

@LinkIsGrim LinkIsGrim merged commit 5d4f1cd into master Sep 11, 2024
4 checks passed
@LinkIsGrim LinkIsGrim deleted the notOpti branch September 11, 2024 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/trivial kind/optimization Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants