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

use make_ in all collection methods #185

Closed
wants to merge 1 commit into from
Closed

use make_ in all collection methods #185

wants to merge 1 commit into from

Conversation

kamicane
Copy link

  • also use lang/isInteger in collection/size and collection/make_ to avoid using array iterators on array-like objects with NaN values
  • removed avoidable slice call in make_
  • all relevant unit tests were updated
  • some objects in unit tests were changed to use string values rather than numeric values: this is due to a shortcoming of jasmine's toEqual() which asserts true for {0: 1} and [1].

fixes #183

 - also use lang/isInteger in collection/size and collection/make_ to avoid using array iterators on array-like objects with NaN values
 - removed avoidable slice call in make_
 - all relevant unit tests were updated
 - some objects in unit tests were changed to use string values rather than numeric values: this is due to a shortcoming of jasmine's toEqual() which asserts true for {0: 1} and [1].
@jdalton
Copy link

jdalton commented May 22, 2014

to avoid using array iterators on array-like objects with NaN values

In terms of array-like detecting I've started favoring one that aligns more with ES6:

// http://people.mozilla.org/~jorendorff/es6-draft.html#sec-tolength
var maxSafeInteger = Math.pow(2, 53) - 1;

// later...
typeof length == 'number' && length > -1 && length <= maxSafeInteger

I find the inconsistency in collection methods confusing with some always returning arrays and others returning objects if you provide an object, some iterating over objects values providing keys and others iterating over object values providing indexes.

Having /array methods specialized for arrays and returning arrays and having /object methods specialized objects and returns objects is great. Collections on the other hand are different. They put priority on array-like over plain object iteration and should return consistent types regardless of collection passed (array, arguments object, string, jquery object, plain object). Also array-like iteration should provide at least value, index, array and plain object iteration value, key, object to callbacks.

@millermedeiros
Copy link
Member

@mout/admin any feedback from you guys? I never use the collection methods, so unsure what would be the best behavior.. in fact I even considered killing the whole collections package a few times.

@conradz
Copy link
Member

conradz commented Aug 6, 2014

I never used the collection methods either, I would be in favor of killing them.

@millermedeiros millermedeiros changed the title use make_ in all collection methods, fixes #183 use make_ in all collection methods Aug 6, 2014
@roboshoes
Copy link
Member

Same here. I don't use them. But oddly enough a lot of times when I talk to people who use AMD structures in their projects but use lodash/underscore I tell them about mout. Usually their first feedback is: "I really like how I can use iterators like forEach on objects and arrays." So, I don't know. Maybe people use them.

/cc @hapticdata

@hapticdata
Copy link
Contributor

I do really like how I can use iterators like forEach on objects and arrays without separate requires's. For me to need to add these individually feels like an unnecessary overhead and foresight.
The types of functions that are in mout/collection I'm often using from underscore or lodash. I don't think of them as critical in mout, but I'm glad they are there. I typically go to mout for the individual utilities that aren't already in my _ object. In my experience with the mout/collection package I've never felt confused by what it returns. But maybe I'm just not looking in these places?

from @jdalton

I find the inconsistency in collection methods confusing with some always returning arrays and others returning objects if you provide an object, some iterating over objects values providing keys and others iterating over object values providing indexes.

What in mout/collection is iterating over an object and returning an object? I feel methods such as map should be returning arrays, like in underscore, and thats how it appears to be.

What methods are iterating over objects and providing indexes instead of keys? That does sound weird but I haven't seen it.

Also array-like iteration should provide at least value, index, array and plain object iteration value, key, object to callbacks.

I completely agree with this, but again I don't see where that isn't happening?

In the end I think letting users get on with their work is more important than the purity of separating objects from arrays. I'm already used to having this from underscore. As is implied by "modular utilities", if I don't want it, I won't use it.

@jdalton
Copy link

jdalton commented Aug 6, 2014

What in mout/collection is iterating over an object and returning an object?

Many collections methods use make_, ex make(arrMap, objMap), which will return an object if its iterating over one. This PR increases the number of methods using make_.

What methods are iterating over objects and providing indexes instead of keys?

See collection/map.

@hapticdata
Copy link
Contributor

I see, you're right mout/collection/map does give me an index instead of a key. That does seem incorrect.
I do also agree with your point related to make_. If I want my object to remain an object I will explicitly use the one in the mout/object package. I use the methods available in mout/collection with an understanding that objects will result in arrays. I choose this as a convenience to reduce the importance of the structure of the input data and to reduce the complexity in how I process it.

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.

Inconsistencies in collection methods
6 participants