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

Removed unneeded conditional #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hjerpbakk
Copy link

No description provided.

@Grinshpon
Copy link

That's not an unneeded conditional. It says if the damage value is zero the function returns zero.

@Berzeger
Copy link

Berzeger commented Jun 2, 2020

Grinshpon, the condition that follows immediately after checks for that again, so I'd say it's unnecessary.

@smbkr
Copy link

smbkr commented Jun 3, 2020

The original implementation could have been an optimisation, if the checks that follow were expensive to perform, allowing the function to return early before incurring that cost -- however this is probably irrelevant in 2020.

@hjerpbakk
Copy link
Author

This is not a performance improvement, but rather a removing of redundant code and brings this check to parity with

if (Special.IsInert || !damage || warhead == WARHEAD_NONE) return(0);

@Sgt-Nukem
Copy link

The original implementation could have been an optimisation, if the checks that follow were expensive to perform, allowing the function to return early before incurring that cost -- however this is probably irrelevant in 2020.

And even now one could get it back by simply swapping !damage to the front and early-out that way:

if ( !damage || Special.IsInert || warhead == WARHEAD_NONE ) return(0);

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.

5 participants