DEV Community

Code Smell 53 - Explicit Iteration

Maxi Contieri on January 05, 2021

We learned loops back in school. But enumerators and iterators are the next generation. TL;DR: Don't use indices while iterating. Prefer Higher l...

Read full post

shadowtime2000 • Jan 5 '21

The Right JS code is kind of wrong syntax wise and stuff. This would be the correct code:


for (color in colors) {
  print(colors[color]);
}

//Closures and arrow functions
colors.foreach(color => console.log(color);

Michel Descoteaux • Jan 5 '21 • Edited

Or in this case it could have been simplified even further to:

for (color of colors) {
  console.log(color);
}

//Closures and arrow functions
colors.forEach(color => console.log(color));

Patrik Jajcay • Jan 5 '21

colors.forEach(console.log);

Maxi Contieri • Jan 5 '21

Nice!

Buğra Hasbek • Jan 5 '21

I don't use js and this confused the hell out of me at first sight :) i was expecting color to be an element in colors but it is used as an index? I think I prefer the c++ way

for (auto color:colors) 
  print(color);

Pacharapol Withayasakpunt • Jan 5 '21

Depends on the programming language; but JavaScript in is damn confusing.
A closing bracket is missing at the end.

Mihail Malo • Jan 5 '21 • Edited

Yeah, unless they wanted to print keys (including inherited), they should have used of not in.
a closing parenthesis*

Maxi Contieri • Jan 5 '21

corrected! thank you!

Pacharapol Withayasakpunt • Jan 5 '21

I also prefer to hide explicit iteration. Still, I had to use the first style for chunking.

const batchSize = 500

for (let i = 0; i < items.length; i += batchSize) {
  await batchInsert(items.slice(i, i + batchSize))
}

The second style (for ... of) is for awaiting inside, or for await ... of.

The third style, I generally use map because of performance, but grammatically, it would be for each. Also, map(async () => ...) can be used with Promise.all (or Promise.allSettled).

Maxi Contieri • Jan 5 '21

In my opinion chinking should be hidden at iteration level.

Pacharapol Withayasakpunt • Jan 5 '21 • Edited

That is, if a convenient library function exists.

Mihail Malo • Jan 5 '21

You want to use of with iterators, in is for string keys of objects (including inherited).
for in and for of syntax in js looks like for (const ident of expression) (where const could be let or var and of could be in)
there's a missing ) in the last statement
most commonly available print fn in js is console.log
Array method is called forEach (capitalization)

so, all in all:

const colors = ['red', 'green']
const {log} = console

for (const color of colors)
  log(color)

// closure/arrow function
colors.forEach(color => log(color))

if for some reason you have to use in to iterate over values of an object, it's typically done like this (to avoid iterating over inherited keys)

for (const key in obj)
  if (obj.hasOwnProperty(key))
    do_thing(key, dict[key])

however you should prefer the new iterators, such as Object.keys(), Object.values() and Object.entries().
As such, the above becomes:

for (const [key, val] of Object.entries(obj))
    do_thing(key, val)

Finally, if you see yourself operating on keys of an Object generically like this, you might instead be looking for Map, which naturally iterates by "entries", and supports non-string keys.

const ref = {two: 2}
const m = new Map([[1, "val1"], [ref, "val2"]])
for (const [k,v] of m)
  console.log("key", k, "value", v)