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

Create for.js #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Create for.js #3

wants to merge 1 commit into from

Conversation

vlazar
Copy link
Contributor

@vlazar vlazar commented Jun 21, 2015

Same as https://github.com/jonschlinkert/array-unique/blob/master/benchmark/code/while.js, but preserves the order of elements in result array, which is nice to have.

Performance is almost as good as while.js

BTW, is there any reason the implementation in https://github.com/jonschlinkert/array-unique/blob/master/index.js is not from https://github.com/jonschlinkert/array-unique/blob/master/benchmark/code/while.js ?

For me on node 0.12.0 array-uniq with Set is the fastest on arrays bigger than 1000 elements. In all other cases https://github.com/jonschlinkert/array-unique/blob/master/benchmark/code/while.js is the fastest.

Same as https://github.com/jonschlinkert/array-unique/blob/master/benchmark/code/while.js, but preserves the order of elements in result array, which is nice to have.

Performance is almost as good as while.js

BTW, is there any reason the implementation in https://github.com/jonschlinkert/array-unique/blob/master/index.js is not from https://github.com/jonschlinkert/array-unique/blob/master/benchmark/code/while.js ?

For me on node 0.12.0 array-uniq with Set is the fastest on arrays bigger than 1000 elements. In all other cases https://github.com/jonschlinkert/array-unique/blob/master/benchmark/code/while.js is the fastest.
@vlazar
Copy link
Contributor Author

vlazar commented Jun 21, 2015

Ah, and the description for https://www.npmjs.com/package/array-unique currently says

Return an array free of duplicate values.

which is confusing, as current implementation from https://github.com/jonschlinkert/array-unique/blob/master/index.js removes elements from the original array.

@jonschlinkert
Copy link
Owner

this is great! thanks for all the hard work and making this lib better! I have some father's day stuff going on today, but I'll review and merge in your pr as soon as I have a chance

@vlazar
Copy link
Contributor Author

vlazar commented Jun 21, 2015

Sure, important stuff first.
Thanks for super quick reply, really appreciate.

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.

2 participants