-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add _.chunk to Array functions. #1919
Changes from all commits
e917a18
db7f405
750f22c
dbc7375
e3035ba
63652f6
a59a6f9
7d4034c
ce3a67a
0c6d140
185abf6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -647,6 +647,23 @@ | |
return range; | ||
}; | ||
|
||
// Split an **array** into several arrays containing **count** or less elements | ||
// of initial array | ||
_.chunk = function(array, count) { | ||
count = count || 1; | ||
|
||
if (count <= 1) return array.slice(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Direct call to slice should instead use slice helper. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Should be |
||
|
||
var result = []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, I wonder if it's faster to compute the length |
||
var i = 0, length = array.length; | ||
|
||
while (i < length) { | ||
result.push(slice.call(array, i, i += count)); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably write line 655-659 as var i = 0, length = array && array.length, j = 0;
while (i < length && count >= 0) {
if (i % count) result[j++] = [];
result[j].push(array[i]);
} (untested). I would be curious to see some benchmarks |
||
|
||
return result; | ||
}; | ||
|
||
// Function (ahem) Functions | ||
// ------------------ | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To support the
count = undefined
case and fix thecount = 1
case, we should default to0
here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the default = 1 makes sense. However do we want to mirror lodash
which makes anything less than 1 mean 1 chunk size?
E.g. _.chunk([1,2,3], 0) => [[1], [2], [3]]
On Tue, Jul 28, 2015 at 11:32 AM, Justin Ridgewell <notifications@github.com
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that behaviour sort of makes sense so that chunk always returns an
array of arrays
On Tue, Jul 28, 2015 at 12:18 PM, Graeme Yeates megawac@gmail.com wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only other reasonable option I see is to return an empty array for
chunk < 1
On Tue, Jul 28, 2015 at 12:19 PM, Graeme Yeates megawac@gmail.com wrote:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine with me, I was just coding the the current tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @jdalton
With v4 coming, nows the time to talk about these details
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the current behavior is correct. If your chunk is less than 1 there is no chunk and a shallow clone is returned.
Think of it like if you had a pie and you cut it into 0 slices you still have a whole pie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A shallow clone isn't returned in lodash. It returns the array in chunks of 1. I think returning a shallow clone would be even worse as the user would have to worry about arrays of arrays and arrays of any.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually leaning with @megawac here. We're not cutting the array into 3 slices, we're gathering into groups of 3. And groups of 0 don't have anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm ok cool.