Skip to content

Conversation

@megawac
Copy link
Collaborator

@megawac megawac commented Jul 21, 2014

Add a slice function with the python contract _.slice(collection, start, end, step)

See https://docs.python.org/2/library/functions.html#slice. Like python slice, it doesn't do any coercing of inputs (besides 0 which is coerced to 1). Python will give an error for invalid inputs, this implementation doesn't bother and carries on. One nice benefit of the step feature is now we can do non destructive reversing _.slice(arr, null, null, -1)

Created some jsperf benchmarks the other day http://jsperf.com/cr-52768

I think it may make sense to add a _.reverse function after this which will also fix a bug _(document.childNodes).reverse(); // Exception

@jdalton
Copy link
Contributor

jdalton commented Jul 21, 2014

As long as the start and end align with Array#slice I'm cool with it.
The addition of step is ok by me too.

it doesn't do any coercing of inputs (besides 0 which is coerced to 1)

???

think it may make sense to add a _.reverse function after this which will also fix a bug

The reverse chaining method is intended for arrays under "Arrays" category so using anything else is undefined territory. I think _.slice may fall into this too as it isn't designed to fix bugs iterating strings in older enviros.

@michaelficarra
Copy link
Collaborator

@megawac: please rebase

@megawac
Copy link
Collaborator Author

megawac commented Aug 5, 2014

Rebased

@michaelficarra
Copy link
Collaborator

👍

@jashkenas
Copy link
Owner

Can you explain why and when you'd want to use this instead of vanilla slice?

@jashkenas jashkenas closed this Aug 6, 2014
@jashkenas jashkenas reopened this Aug 6, 2014
@jdalton
Copy link
Contributor

jdalton commented Aug 6, 2014

The primary reason for me was the vanilla slice can return sparse arrays, the util version will return dense (which is what other Underscore methods return). There's a bonus of small/medium collections being sliced faster, supporting nodelists in IE < 9, and Graeme's addition of the step param too.

@akre54 akre54 mentioned this pull request Sep 9, 2014
8 tasks
@megawac megawac added this to the v2.0 milestone Sep 9, 2014
@jashkenas jashkenas removed this from the v2.0 milestone Dec 11, 2014
@megawac megawac removed this from the v2.0 milestone Dec 11, 2014
@megawac megawac mentioned this pull request Jan 19, 2015
16 tasks
@jashkenas
Copy link
Owner

Are there any use cases for step outside of reversing?

@jridgewell
Copy link
Collaborator

http://jsperf.com/cr-52768/6

I think this is trying to do too much. Can we make it a simple, dense version of Array#slice?

@megawac
Copy link
Collaborator Author

megawac commented Feb 13, 2015

@jashkenas from my experience, the step arg is most commonly used for reversing but it can also be useful for picking every 2nd element. I'm not sure if thats useful enough for us to entertain though (was fun to implement though)

@jgonggrijp
Copy link
Collaborator

Not all review comments were addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants