My Strange Code#1 map の使い方がおかしい(JavaScript)

2021年4月18日

需要がないとは思いますが、自分が過去に書いたコードを見返してみておかしなコードを直してみます。

商品の数量算出処理

以下は商品の処理数を算出する関数です。

const getProcessedNum = list => {
  let num = 0
  list.map(product => {
    num =
      num +
      (Number(product.orderacceptQuantity) +
        Number(product.keepQuantity) +
        Number(product.reserveQuantity))
  })
  return num
}

mapの使い方がおかしい

このコードではlist.mapというようにmapを使っています。

しかし、戻り値が使われていません。

mapは本来は新しい配列を作成するためのものですが、上記のコードでは単にループさせたいがために使っているように見えます。

これであればforEachの方が適切です。

もっと言えば、reduce を使うことで num 変数を削除することができます。

以下が変更後です。

const getProcessedNum = list => (
  list.reduce((acc, product) => (
    acc + (
      Number(product.orderacceptQuantity) +
      Number(product.keepQuantity) +
      Number(product.reserveQuantity)
    )
  ), 0)
)

パラメータ名や関数名を改善してみる

getProcessedNum でも意味がなんとなく分かりますが、orderacceptQuantity、keepQuantity、reserveQuantity のように Quantity を使っているので、getProcessedQuantity とした方が統一感がでます。

また、パラメータのlistはproductsとした方が、どのようなデータが引数になるのかを予想しやすくなります。

以下のように変更しました。

const getProcessedQuantity = products => (
  products.reduce((acc, product) => (
    acc + (
      Number(product.orderacceptQuantity) +
      Number(product.keepQuantity) +
      Number(product.reserveQuantity)
    )
  ), 0)
)

その他

他に気になることとしては、product.orderacceptQuantity、product.keepQuantity、product.reserveQuantity が undefined になるかのチェックをしていない点です。

Number(undefined) は NaN になるので、チェックは必要そうな気がします。

もしかしたら、undefined になることはないのかもしれませんが、今見るとそれがわかりません。(TypeScript を使っていたら容易にわかるのですが)