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

Relax polymorphism checks in static methods #44

Merged
merged 2 commits into from Jan 28, 2014
Merged

Relax polymorphism checks in static methods #44

merged 2 commits into from Jan 28, 2014

Conversation

kriskowal
Copy link
Member

These changes should improve the performance of polymorphic checks and
equality comparison in general. A method is no longer required to be on
the prototype, which was an unrealistic expectation of JavaScript in the
wild, but retains the ability to deeply compare object literals. The
code for comparing object literals will be exercised less frequently due
to additional checks to bail out early.

I could not bear to completely remove support for comparing object literals because equals should be useful down the line for a unit testing or assertion library and deep comparison of object literals is an expected behavior of equals as opposed to be or is.

These changes should improve the performance of polymorphic checks and
equality comparison in general. A method is no longer required to be on
the prototype, which was an unrealistic expectation of JavaScript in the
wild, but retains the ability to deeply compare object literals. The
code for comparing object literals will be exercised less frequently due
to additional checks to bail out early.
Should be true, otherwise heaps can't get rid of NaN.

Thanks @ducarroz.
@aadsm
Copy link
Contributor

aadsm commented Jan 25, 2014

How about the array, could it be worthwhile to just rely on contentEquals and default to Object.is?

@kriskowal
Copy link
Member Author

Consider:

expect(foo).equals([{index: 10}, {index: 20}, {index: 30}]);
expect(foo).equals([["a", 10], ["b", 20], ["c", 30]]);

I do think we have to retain equals as the default behavior. We can override contentEquals and contentCompare for specific arrays and equals and sorted will respect the change.

@aadsm
Copy link
Contributor

aadsm commented Jan 25, 2014

I guess I'm happy with that, but I was expecting the other way around. I would assume that in that case, where you want a custom comparison (other than ===), you would define the contentEquals; but by default we'd have === comparison.

kriskowal added a commit that referenced this pull request Jan 28, 2014
Relax polymorphism checks in static methods
@kriskowal kriskowal merged commit e4849dc into montagejs:master Jan 28, 2014
@kriskowal kriskowal deleted the equivalence branch January 28, 2014 19:23
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.

None yet

2 participants