davidmerfield / davidmerfield/Typeset

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

Open
#40 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
2.7k
Forks
53
PR merge metrics
No merged PRs in 30d

Description

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.

``` javascript
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!

Contributor guide

No contributing guide indexed for this repository

Research direction

Add Array.prototype.foo = () => 'bar' to index.js, then run the test suite to expose every place affected by for...in array iteration. Replace those array iterations with an appropriate loop form and confirm the suite passes without enumerating the added prototype property.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
web-dev
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.