refactor(Angular): make NaN check in date equality cleaner

[shahata]

No description provided.

[caitp]

I don't think recursion is really any better here. Refactoring this so that we can use one NaN-check without recursion would be better

[shahata]

It's not really recursion if it is always invoked only one time... Whatever you like, I thought this is what you were aiming at with your comment.

[caitp]

It's not going to recurse indefinitely, but it is actually recursion --- I do think we can improve it though --- you don't have to close this though, it shouldn't take too much to come up with something better.

[caitp]

http://jsperf.com/ng-equals-dates-nan from a perf check the recursion option performs a bit better in v8, my guess is because of fewer calls/invocations --- but it might make sense to land that version anyway. You should re-open this

[shahata]

yes, difference might be a bit smaller if the original invocation didn't invoke getTime() twice unnecessarily, but imo this is cleaner than fixing the duplicate invocations.

[caitp]

I would actually still rather just have a quick and dirty "CompareNumbers()" routine that should be NaN aware, and invoke that rather than invoking equals() again --- chances are good we won't accidentally break this, but it doesn't take too big of an accident to go from recursing once and recursing indefinitely, and those are always a bit annoying

[caitp]

http://jsperf.com/ng-equals-dates-nan we do slightly better with the compareNumbers() version --- since it doesn't have to do very much work in order to compare them, honestly I would prefer something like that.

See what one other person thinks though I guess (it's worth noting that this is definitely not a frequently evaluated section of code, so the performance doesn't matter a whole lot here, I just like writing these tests)

[IgorMinar]

I think this change is good as is

[IgorMinar]

I'm going to merge it in

[caitp]

sgtm