All equals methods should first check for reference equality... #84
Comments
|
I know it is good practice in C++ when implementing |
|
I think it's the same reason you would want to do it on Matrix4. If two Cartesian2 values are identical, then it requires 4 property look-ups in order to determine equality. So doing reference equality first is faster (plus the reference equality check itself has no performance impact). It also makes the code all follow the same pattern. I heard someone say once that "consistency is clean". |
|
Right, I was thinking about saying do it for consistency too. So it's an optimization - assuming that we often compare objects whose references are the same. Why doesn't the reference equality have a performance impact? My C++ answer is because the branch predictor always gets it right, but is there a JavaScript reason? I actually don't care all that much one way or the other, I just want to confirm that we thought through it. |
1. All prototype functions now have static equivalents. 2. All functions now do proper error checking. 3. All functions no longer require Cartesian3 instances and instead will work with any object that has an x, y, and z properties. 4. All functions that produce a Cartesian3 now take an optional result parameter. 5. Code coverage for Cartesian3 is now 100% 6. Remove a few unused functions. 7. Add several functions to Cartesian2 that already existed on Cartesian3 (everything but cross). These changes are related to issues #71, #84, #86 (those issues will remain open until we address them for every Core type).
1. All prototype functions now have static equivalents. 2. All functions now do proper error checking. 3. All functions no longer require Cartesian4 instances and instead will work with any object that has an x, y, z, and w properties. 4. All functions that produce a Cartesian4 now take an optional result parameter. 5. Code coverage for Cartesian4 is now 100% 6. Remove Math.angleBetween because it already exists on the Cartesian functions. These changes are related to issues #71, #84, #86 (those issues will remain open until we address them for every Core type).
|
I'm fine with calling this cleanup "done" There may be a few straggling cases that we'll uncover as we continue to do clean-up, but all of the "important" core types certainly have the reference check. |
While the performance improvement will probably be negligible, it's just good practice. Static instances should also handle the case where both parameters are undefined and prototype instances should check if the passed in object is undefined.
The text was updated successfully, but these errors were encountered: