Help us understand the problem. What is going on with this article?

最近見かけたイケてないコード【解決編】

More than 1 year has passed since last update.

【前回(最近見かけたイケてないコード)】 の続き

なぜ「イケてない」のか?

パターンの数だけ即値を返すreturn文を記述したコードの何がイケてないのか考える。

まず「マジックナンバー」を躊躇なく使ってしまうのがイケてない。
そもそも、パターン数24に対して、返す値は 7, 5, 3, 1 の4種類しかないので、その意味するところが重複している可能性が高い。
「マジックナンバーを使うな」という戒めは、それらの値の真に意味するところを「よく考えろ」ということ。

意味を考えないから、本質的には同じ処理を「集約しよう」とも思わない。
もし突然の仕様変更が来たとき、複数箇所に記述された「同じ処理」を一つ一つ書き替えるなど、「怠惰」を旨とするプログラマにあるまじき行為だというのに。

どう直すか

どこまで「意味」を見出せるかは人によって変わってくるが、自分ならこうするという例を挙げておく。

最長の「7営業日先」を返すパターンとそれ以外を見比べると、前者はリージョンの東西に関わらず「利用(ABとも付加)」を含み、後者は含まないことが分かる。
推察するに、東西で準備作業は並行して進められるが、一番日数の掛かる「利用(ABとも付加)」がボトルネックとなる、ということだろう。
同様に「5営業日先」を返すパターンとそれより短い「3営業日先」「1営業日先」を返すパターンを見比べれば、前者はリージョンの東西に関わらず「利用(Aのみ付加)」を含み、後者は含まない。
「3営業日先」を返すパターンと「1営業日先」を返すパターンを比較すれば、前者がリージョンの東西に関わらず「利用(Bのみ付加)」を含み、後者は含まないこともすぐに分かる。

東西それぞれで取り得る状態は下記の5種類だが、東西とも「利用なし」というパターンは存在せず、今回に限れば「利用(付加なし)」と「利用なし」を区別する必要は無さそうだ。

  • 利用(ABとも付加)
  • 利用(Aのみ付加)
  • 利用(Bのみ付加)
  • 利用(付加なし)
  • 利用なし

方針としては

  1. 「状態」を4種類に分け、それぞれに名前と値を持たせる
  2. リージョンの東西ごとの状態を判定し、値に変換する
  3. 東西の値を比較し、より日数の掛かる方を全体の結果として返す

コード例は若干jsっぽく書いてみたが、本稿の主題とは関係ない。

/*
 * 全体として必要な日数を返す関数
 * @public
 * @returns {number} - 全体として必要な日数
 */
function getNecessaryDays() {
  // 状態の名前と値を定義
  const NECESSARY_DAYS = {
    BOTH_A_AND_B: 7,
    A_ONLY: 5,
    B_ONLY: 3,
    NEITHER: 1,
  };
  const necessaryDaysEast = getNecessaryDaysOf('east');
  const necessaryDaysWest = getNecessaryDaysOf('west');

  return Math.max(necessaryDaysEast, necessaryDaysWest);

  /*
   * リージョンの東西を与えて必要な日数を返す関数
   * @private
   * @param {string} region - リージョン名(east|west)
   * @returns {number} - 必要な日数(営業日)
   */
  function getNecessaryDaysOf(region) {
    // ※実装は「状態」の持ち方によるのでテキトー
    if (state[region].optionA) {
      if (state[region].optionB) {
        return NECESSARY_DAYS.BOTH_A_AND_B;
      }
      return NECESSARY_DAYS.A_ONLY;
    }
    if (state[region].optionB) {
      return NECESSARY_DAYS.B_ONLY;
    }
    return NECESSARY_DAYS.NEITHER;
  }
} 

24回 return文を書き殴るよりだいぶ人がましいコードになったと思うがどうか?

「7営業日先」を返すパターン

No. 西 開始見込み
1 利用(ABとも付加) 利用(ABとも付加) 7営業日先
2 利用(ABとも付加) 利用(Aのみ付加) 7営業日先
3 利用(Aのみ付加) 利用(ABとも付加) 7営業日先
5 利用(ABとも付加) 利用(Bのみ付加) 7営業日先
7 利用(ABとも付加) 利用(付加なし) 7営業日先
9 利用(Bのみ付加) 利用(ABとも付加) 7営業日先
11 利用(付加なし) 利用(ABとも付加) 7営業日先
17 利用(ABとも付加) 利用なし 7営業日先
21 利用なし 利用(ABとも付加) 7営業日先

「5営業日先」を返すパターン

No. 西 開始見込み
4 利用(Aのみ付加) 利用(Aのみ付加) 5営業日先
6 利用(Aのみ付加) 利用(Bのみ付加) 5営業日先
8 利用(Aのみ付加) 利用(付加なし) 5営業日先
10 利用(Bのみ付加) 利用(Aのみ付加) 5営業日先
12 利用(付加なし) 利用(Aのみ付加) 5営業日先
18 利用(Aのみ付加) 利用なし 5営業日先
22 利用なし 利用(Aのみ付加) 5営業日先

「3営業日先」を返すパターン

No. 西 開始見込み
13 利用(Bのみ付加) 利用(Bのみ付加) 3営業日先
14 利用(Bのみ付加) 利用(付加なし) 3営業日先
15 利用(付加なし) 利用(Bのみ付加) 3営業日先
19 利用(Bのみ付加) 利用なし 3営業日先
23 利用なし 利用(Bのみ付加) 3営業日先

「1営業日先」を返すパターン

No. 西 開始見込み
16 利用(付加なし) 利用(付加なし) 1営業日先
20 利用(付加なし) 利用なし 1営業日先
24 利用なし 利用(付加なし) 1営業日先
Nyagoking
8ビットPC全盛期を知るアラフィフプログラマ。 COBOLやらFORTRANやらやってましたが、今はクラスチェンジして主にフロントエンド側に携わってます。 訳も分からずコピペコード書き散らす輩が苦手。
Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away