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

Prefer Set.prototype.values() over .entries() #131

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

Conversation

EvanHahn
Copy link

@EvanHahn EvanHahn commented Sep 9, 2021

Each iteration value in Set.prototype.entries() is an array of two identical values:

const mySet = new Set(['a' , 'b', 'c']);
console.log([...mySet.entries()]);
// => [ [ 'a', 'a' ], [ 'b', 'b' ], [ 'c', 'c' ] ]

That's unnecessary for this library, which can just use Set.prototype.values().

const mySet = new Set(['a' , 'b', 'c']);
console.log([...mySet.values()]);
// => [ 'a', 'b', 'c' ]

This simplifies the code a bit and should result in a small speed improvement.

Each iteration value in [`Set.prototype.entries()`][0] is an array of
two identical values:

    const mySet = new Set(['a' , 'b', 'c']);
    console.log([...mySet.entries()]);
    // => [ [ 'a', 'a' ], [ 'b', 'b' ], [ 'c', 'c' ] ]

That's unnecessary for this library, which can just use
[`Set.prototype.values()`][1].

    const mySet = new Set(['a' , 'b', 'c']);
    console.log([...mySet.values()]);
    // => [ 'a', 'b', 'c' ]

This simplifies the code a bit and should result in a small speed
improvement.

[0]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/entries
[1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/values
@EvanHahn
Copy link
Author

Let me know if there's anything I can do to help this PR along.

@EvanHahn
Copy link
Author

This PR has been open for two years. Anything I can do to help get this merged?

@master-elodin
Copy link

sorry this hasn't been merged yet (if I had any power I would), but it did help me out a lot. thanks!

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