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

_.eq redundant comparison when both null #2629

Closed
ahonn opened this issue Nov 21, 2016 · 6 comments
Closed

_.eq redundant comparison when both null #2629

ahonn opened this issue Nov 21, 2016 · 6 comments

Comments

@ahonn
Copy link
Contributor

ahonn commented Nov 21, 2016

in _.eq function:

 eq = function(a, b, aStack, bStack) {
    // Identical objects are equal. `0 === -0`, but they aren't identical.
    // See the [Harmony `egal` proposal](http://wiki.ecmascript.org/doku.php?id=harmony:egal).
    if (a === b) return a !== 0 || 1 / a === 1 / b;
    // A strict comparison is necessary because `null == undefined`.
    if (a == null || b == null) return a === b;
   ......
}

a === b is true when a = null and b = null, so the function will return true.
and then, if (a == null || b == null) is true, return a === b.
It would never be true, why not directly return false:

if (a == null || b == null) return false;

I checked the #2180 and #2179 , but I still don't understand why not directly return false?

@jridgewell
Copy link
Collaborator

Yah, looks like we could. Want to open a PR?

@ahonn
Copy link
Contributor Author

ahonn commented Nov 21, 2016

Nice, I have opened a PR #2630

@bnjmnt4n
Copy link
Contributor

bnjmnt4n commented Nov 21, 2016

@ahonn since we are using a == null || b == null, a and b could be null or undefined due to == not being a strict equal comparison. Hence a === b is needed to ensure that a and b are not regarded as the same, if one is null and the other is undefined.

@jridgewell
Copy link
Collaborator

Yes, but we've already done a a === b check above it. So, if either a or b is null or undefined, we know that both aren't.

@bnjmnt4n
Copy link
Contributor

Yes, but we've already done a a === b check above it

Ah, I completely missed that :| Thanks @jridgewell!

@ahonn
Copy link
Contributor Author

ahonn commented Nov 21, 2016

When running if (a == null || b == null) ..., a === b would never be true.
Because if a === b is true, will return a !== 0 || 1 / a === 1 / b. So, That must be false.

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

No branches or pull requests

4 participants