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

Recommendation against use of for...in for array iteration. #40

Open
noahlange opened this issue May 12, 2016 · 0 comments
Open

Recommendation against use of for...in for array iteration. #40

noahlange opened this issue May 12, 2016 · 0 comments

Comments

@noahlange
Copy link

Howdy, love the library.

Running into some issues regarding your use of the for...in statement for array iteration. for...in is for enumerable properties of collections, not iteration through iterables (e.g. Arrays). I'd recommend using array.forEach(), for...of (broadly supported by Node >= 0.12, dunno about browser compat.) or a standard for loop, which is obviously more verbose but more appropriate than for...in.

for...in will throw when some dingus (not me, but there are lots of dinguses out there) modifies the Array prototype, because for...in will kick back any additional enumerable properties, not just the elements of the array.

Copy-pasta'd some relevant code from the MDN demonstrating the issue.

Object.prototype.objCustom = function () {}; 
Array.prototype.arrCustom = function () {};

let iterable = [3, 5, 7];
iterable.foo = "hello";

for (let i in iterable) {
  console.log(i); // logs 0, 1, 2, "foo", "arrCustom", "objCustom"
}

for (let i of iterable) {
  console.log(i); // logs 3, 5, 7
}

Add Array.prototype.foo = () => 'bar' to index.js and run the test suite. It'll run through all the places where that needs to be changed.

Cheers!

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

No branches or pull requests

1 participant