0

I have an object:

things = {
  table: 'red',
  chair: 'green'
}

Adhering to Airbnb's JavaScript Style Guide, I want to create a function that duplicates this object, setting all keys of the object to blue.

My 2 options seem to be:

1 Use reduce

function alwaysBlue(things) {
  return Object.keys(things)
    .reduce((acc, thing) => ({ ...acc, [thing]: 'blue' }), {})
}

2 Use forEach

function alwaysBlue(things) {
  const blueThings = {}
  Object.keys(things)
    .forEach(thing => (blueThings[thing] = 'blue'))
  return blueThings
}

(1) deconstruction on every iteration seems expensive, but if I'm to take no-param-reassign in to account I can't just append to the accumulator on each iteration

However, forEach (2) should be avoided in favour of map() / every() / filter() / find() / findIndex() / reduce() / some(), according to https://github.com/airbnb/javascript#iterators--nope

So which is the preferred approach if I'm to adhere to Airbnb's JavaScript Style Guide - or is there another approach I'm missing?

Alasdair McLeay
  • 2,572
  • 4
  • 27
  • 50

5 Answers5

2

I'd say the general guideline would be that you can adhere to your coding conventions of choice unless they force you into writing implementations that have a negative impact on performance that is big enough to consider. If that's the case then you will have to make the necessary sacrifice of coding standards in order to write an implementation that performs well enough.

I do not know what is your real-world data, but I bet #1 would do just fine and if not then #2 would most likely be a good compromise. The guide doesn't say forEach is bad, it's just not best.

plalx
  • 42,889
  • 6
  • 74
  • 90
1

Answered on the github issue (which should be the first and only place you go for questions on this guide):


Since that's invalid syntax, I assume your object is:

   things = {
      table: 'red',
      chair: 'green'
    }

Your first example, with the reduce, is correct. "Seems expensive" is something that you shouldn't worry about - performance is the least important thing, only to be concerned about after code is correct, clean, tested, and profiled.

Only if it ends up being necessary (as determined by fully benchmarking your app, not by microbenchmarks like jsperf), then you'd use your forEach approach next, and then if it still was necessary, you'd devolve it to a for loop.

LJHarb
  • 32,486
  • 2
  • 32
  • 38
0

If you're up for using a third party tool like lodash you can use mapValues:

_.mapValues(things, () => 'blue');
lobati
  • 9,284
  • 5
  • 40
  • 61
0

You can use Object.assign, which doesn't create a new object, but rather just appends the item to the accumulator. Would be better for performance and adheres to the style guide.

Object.keys(things)
    .reduce((acc, thing) => Object.assign(acc, {[thing]: 'blue' }), {})
Karamell
  • 1,023
  • 9
  • 23
-2

Just use a dead simple loop:

function alwaysBlue(things) {
  const blueThings = {}
  for (const key in things) // or `for (const key of Object.keys(things))`
    blueThings[key] = 'blue'
  return blueThings
}

Those iteration methods are only good for arrays, which we aren't dealing with here.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Please explain what's wrong with this or how it doesn't adhere to the style guide – Bergi May 18 '17 at 23:15
  • I did not downvote, but they specifically mention that for in or for of must be avoided. – plalx May 19 '17 at 01:54
  • @plalx The style guide says "prefer", not "must be avoided". `for in` does exactly the same thing as `Object.keys(…).forEach(…)`, but it's simpler, more readable, faster, and makes it more obvious that the body is used for mutation. Which doesn't matter a lot since `alwaysBlue` as a whole still is pure. But yeah, when one doesn't care about quadratic complexity and is fine with experimental syntax, the pure `reduce` solution is the way to go. – Bergi May 19 '17 at 02:53
  • I didn't downvote, however for..in is marked as restricted syntax in the accompanying eslint settings: https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/rules/style.js "message: 'for..in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries}, and iterate over the resulting array.'," – Alasdair McLeay May 22 '17 at 16:20
  • The link above to 11.1 also states "Don't use iterators... Use... Object.keys() / Object.values() / Object.entries() to produce arrays so you can iterate over objects." – Alasdair McLeay May 22 '17 at 16:22
  • @AlasdairMcLeay Ah, thanks. However it really should be "…, which virtually never contains enumerable properties, so it doesn't matter a lot". – Bergi May 22 '17 at 17:21