Zenn
💡

TypeScript: 指定回数反復処理の読み手にやさしい記述

2024/08/13に公開
JavaScript
TypeScript
リファクタリング
tech

やりたいこと

  • for文よりもすっきりと書きたい
    n回の処理結果ごとの要素からなる配列を作りたい場合に、宣言・for文で複数行使いたくない。
    簡潔に書く方法はないか？
  • 可読性は下げたくない
    Array(n).map() で書かれた他人のコードを見てぱっと見でミスに気づけなかった。
    本流と関係のない挙動を把握するために脳のリソースを割かずに済むようにしたい。

結論

  • 指定要素数の配列を作成する関数を定義して使用する。
  • 「中にArray(n)を閉じて見えないようにする」「振る舞いについてJSDocに残す」ことで、可読性を下げない・読み手に無駄な負荷をかけないような優しい実装になる。
/**
 * 指定した要素数の配列を返す
 * @param {number} length 作りたい配列の要素数 正の整数
 * @returns 各要素がundefinedの配列
 */
const createArray = (length: number) => {
  if (length <= 0 || !Number.isInteger(length)) {
    throw new Error(`Error createArray requires positive integer`);
  }
  return [...Array(length)]
}

// 呼ぶ側
const res = createArray(5).map((_, i) => i * 10)

理由・背景

想定した挙動と異なるコード

コードレビュー中に下記のようなコードに出会いました。

const arr = Array(5).map((_, i) => i * 10);
arr.forEach((i) => console.log(i));

「指定した要素数の配列を作って、それぞれにやりたい処理を行うのね。ふむふむ。」と流してmerge。その後、forEach内の処理がうまく動いていないことが発覚。（そもそも正常系の挙動を確認したかのチェックが漏れているというお話も、、、）
結論、正しい記述の一例としてはArray(5) となっているところが [...Array(5)] であるべきでした。

問題点：Array(n), empty の挙動がわかりにくい

知っておけよと言われたらそれまでですが、直感的には把握しにくい仕様となっている気がします。

const arr = Array(5);
console.log(arr);    // [empty × 5]
arr.forEach(i => console.log(i)); // 何も表示されない

const arr2 = [...Array(5)]
console.log(arr2) // [undefined, undefined, undefined, undefined, undefined]
arr2.forEach(i => console.log(i)); // undefined が5回表示される

Array(n)で作られた配列の要素emptyは、undefinedとは異なります。
forEachやmapではスルーされ、mapでは返ってきた配列でもemptyのままとなります。

全体の処理を追う中で、それに気付けるか、正しいかどうかを判断する必要がでてくるとノイズになります。

想定した挙動となるコード

参考：Is there a mechanism to loop x times in ES6 (ECMAScript 6) without mutable variables?

const arr = [...Array(5)].map((_, i) => i * 10); // 例1
const arr = Array(5).fill(0).map((_, i) => i * 10); // 例2
const arr = [...Array(5).keys()].map((i) => i * 10); // 例3

// 例1~3
console.log([...arr.keys()]); // [0, 10, 20, 30, 40]

などなどいろいろありますが、
スプレッドで展開するとかfill()で埋めるとかkeys()はemptyをスルーせずにundefinedと同様に扱うとか、読み手に求められるものが多いため避けたい思いがあります。

解決策

自前で関数を用意して、全体の処理を追うときにはその中身を考えないようにする形で回避するのがまるいです。
JSDocを記述することで使用者も読み手も処理全体で何をしているかのみ意識すればよく、emptyとなっているか・emptyをどう扱っているかを気にする必要がなくなります。

ややこしい実装は見えないようにする

/**
 * 指定した要素数の配列を返す
 * @param {number} length 作りたい配列の要素数 正の整数
 * @returns 各要素がundefinedの配列
 */
const createArray = (length: number) => {
  if (length <= 0 || !Number.isInteger(length)) {
    throw new Error(`Error createArray requires positive integer`);
  }
  return [...Array(length)]
}


// 呼ぶ側
const res = createArray(5).map((_, i) => i * 10)

呼ぶ側では、内部実装は気にする必要はなく
共通化した関数で何か不具合があるとなった際には、配列を作る処理がどうなっているか？に注意してコードを読むので、僕のレビューのような見落としは起こりにくくなります。

使いやすくする

関数化した場合のデメリット・さらなる改善点もあります。
あたりまえですが、

  • 汎用性を持たせないとほしい形に応じて関数化が必要になる
  • 機能を持たせないと使いにくい

ということです。

両者を同時に実現するのは難しいです。
汎用性を持たせないと複数の関数を用意する必要がありますが、
機能に乏しいと、呼び出したあとにいろいろと処理が必要になり、せっかく読みやすくするために用意した関数がかえって可読性を下げる要因になってしまいます。

  • 汎用性UP
    • Iteraterを返す
  • 機能性UP
    • 初項、公差、要素数を指定できるようにする
    • 初期値(=配列の型)を指定できるようにする

などなどいろいろな関数が考えられますが、僕の経験上開発中にいちばん使いやすいのは、undefinedの配列で返す例です。
機能は乏しいですが、開発や運用の中である程度用途が絞られるのであれば、最終的にリファクタして共通関数側に機能を閉じていく形が進めやすい印象です。

まとめ

  • 可読性を損なわないように関数化し、読みにくいものは内部に閉じる
  • JSDocで使い手・読み手に知ってほしいことを見えるようにする
  • そのうえで汎用性を損なわない程度に機能を持たせる

Discussion