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

Added default every/all iterator as _.identity for consistency with any/some #661

Merged
merged 1 commit into from
Aug 31, 2012

Conversation

lennym
Copy link
Contributor

@lennym lennym commented Jun 28, 2012

It struck me as odd that _.all needed an iterator to be defined, but _.any defaulted to identity. It seems to me that these methods should be consistent with one another.

@braddunbar
Copy link
Collaborator

Hey @lennym, thanks for your pull request! This has actually been discussed before in #218. Since ES5 specifies that the iterator is not optional and Underscore provides a fallback, we should stick to being compatible with the upstream API.

If IsCallable(callbackfn) is false, throw a TypeError exception.

@braddunbar braddunbar closed this Jun 28, 2012
@lennym
Copy link
Contributor Author

lennym commented Jun 28, 2012

I assume the default iterator in _.any is just for backwards compatibility then, since ES5 some also requires an iterator?

It just seems slightly odd to (as mentioned in #218) break backwards compatibility in favour of upstream compatibility in one instance and not another.

@braddunbar
Copy link
Collaborator

Good point, I should've looked closer before dismissing as a duplicate. :)

@jashkenas Any reason not to mirror any/some here?

@braddunbar braddunbar reopened this Jun 28, 2012
@jdalton
Copy link
Contributor

jdalton commented Jun 28, 2012

Any reason why my comment was deleted?

It was smth like

The whole ES5 thing is beside the point because this is done before the native method call. I think it's odd Underscore will hold up spec on non-consistency issues like this but throw it under the bus for others like sparse array support where there is actually a real consistency issue between native vs. fallback implementations.

To which @braddunbar replied "Good point, I should've looked closer before dismissing as a duplicate. :)".

@jashkenas
Copy link
Owner

Yep -- having the iterator default to identity is pretty damn useful. Merging.

jashkenas added a commit that referenced this pull request Aug 31, 2012
Added default every/all iterator as _.identity for consistency with any/some
@jashkenas jashkenas merged commit 50d4959 into jashkenas:master Aug 31, 2012
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.

4 participants