読みやすいコード、メンテナンス性の良いコードを書こう

エンジニアの松永と申します。

私は開発リーダー的な役回りが多いので、コードレビューをする機会が多いです。いろんなプロジェクトを経験して実体験として感じるのは、煩雑で分かりにくい設計・コードはやっぱりバグが多いです。ですので、分かりやすいコード、メンテナンスしやすいコードとはどんなものかについて関心があります。

私の好みの要素もありますが、私が心がけていることを書き連ねます。

なお、インデントをTabにするかSpaceにするかや、{} の位置はどこにすべきかなどの狭義のコーディングスタイルには触れません。

似た処理のコピペはできるだけ避ける

関数等の処理ブロックがあり、その処理ブロックの一部だけ異なる処理を実装するとします。

この時、処理ブロックをコピペして異なる処理の部分だけ変更するのはお勧めできません。処理ブロックをコピペしてしまうと、将来の改修で処理を追加する際、コピペ元とコピペ先の両方の処理ブロックに追加しなければなりません。また、コピペの処理ブロックが存在することを知らない人が改修する場合に漏れやすいです。処理ブロックまるごとコピペするのではなく、処理の大部分を共通化して異なる部分だけ条件分岐するのが良いと思います。

また、3つ目の「一部だけ異なる処理」を実装する事になった時、元がコピペで済ませてあると3つ目もコピペ対応で済ませる人が多いのではないかと思います。処理を共通化すると既存の処理に影響が出るので避けようとする意識が働くはずです。そして、どんどんコードが煩雑化していくと思います。

ただし、リリース間際のバグ修正等で、修正の影響を限定的にする方を優先すべき等の事情はあるので、状況に応じての判断が必要です。

概要から書く

ソースコードも言語です。人が読むケースがある以上、自然言語のライティングテクニックと同様に、まず概要・全貌が分かる処理を書き、関数の中に詳細を書くのが良いと思います。

例えば、以下のように条件分岐が列挙するケースですが、各条件の中のコードが短ければ問題は無いと思いますが、長い場合だと見通しが悪くなります。

func() {
  if (条件判定1) {
    // そこそこ長い処理
  } else if (条件判定2) {
    // そこそこ長い処理
  } else {
    // そこそこ長い処理
  }
}

こういう場合は、処理の部分を関数化することで条件分岐の全貌をスクロールせずに把握できるので見通しが良くなり、分かりやすくなると思います。

func() {
  if (条件判定1) {
    funcA()
  } else if (条件判定2) {
    funcB()
  } else {
    funcC()
  }
}

funcA() {
  // そこそこ長い処理
}

funcB() {
  // そこそこ長い処理
}

funcC() {
  // そこそこ長い処理
}

ひとつのことをちゃんとやるようにパーツ化する

機能Aに属する処理が以下の通りあるとします。

  処理A-1-1
  処理A-1-2
  処理A-2-1
  処理A-2-2

関数化する際、以下のように書くこともできますが、深いところでいろいろやると使い回しが効かなくなります。 別のところで 処理A-1 だけを実行したくなっても、funcA-1() 内の funcA-2() が要らないので funcA-1() を流用することができません。

funcA() {
  funcA-1()
}

funcA-1() {
  処理A-1-1
  処理A-1-2
  funcA-2()
}

funcA-2() {
  処理A-2-1
  処理A-2-2
}

処理A-1、処理A-2 の処理を切り離し可能なら、次のようにすることで funcA-1() を別のところでも使えます。 また、funcA() は 処理A-1 と 処理A-2 をやるのだと見通しが良くなります。

funcA() {
  funcA-1()
  funcA-2()
}

funcA-1() {
  処理A-1-1
  処理A-1-2
}

funcA-2() {
  処理A-2-1
  処理A-2-2
}

ネストは浅く

ネストしているということは、ネストの開始位置と終了位置があるはずで、多くは何らかの条件分岐によるものです。

ネストが深い、かつそこそこ長いコードだと、コードを追っているうちに「今見ているコードは何と何の条件の組み合わせの処理だっけ？」と分からなくなってしまうことがあります。処理が長くなるのは致し方ないとしても、ネストが浅ければ条件を覚えていられる事が多くなると思います。ですので、ネストの浅い方が分かりやすくミスが少なくなると思います。

例1
elseを乱用せず、エラー処理などの「本筋ではない処理」を早めにreturnすれば、関数内の本筋の処理が何なのか、どこに書いてあるのかが分かりやすくなると思います。

例えば以下のように条件分岐の数だけネストする書き方だと、本筋の処理がどれなのか分かりづらいです。

func() {
  if (エラーチェック == true) {
    if (条件判定 == true) {
      // 本筋の処理
    } else {
      // その他の処理
    }
  } else {
    // エラー時の処理
  }
}

こういう場合は、以下のように本筋ではない処理を先に検出して早々にreturnすれば、elseを書く必要が無くなるのでその分ネストを少なくできます。

func() {
  if (エラーチェック == false) {
    // エラー時の処理
    return;
  }

  if (条件判定 == false) {
    // その他の処理
    return;
  }

  // 本筋の処理
}

例2
仕様レベルで多くの条件判定が必要な場合は実装の条件分岐も多くなります。

func() {
  if (条件判定1) {
    if (条件判定2) {
      if (条件判定3) {
        // そこそこ長い処理
      }
    }
  }

  if (条件判定4) {
    for (条件判定5) {
      // そこそこ長い処理
    }
  }
}

この場合は、ネストする処理の部分を関数化すると、見通しが良くなるし、ネストをリセットできます。

func() {
  if (条件判定1) {
    if (条件判定2) {
      if (条件判定3) {
        funcC()
      }
    }
  }

  if (条件判定4) {
    for (条件判定5) {
      funcB()
    }
  }
}

funcB() {
  // そこそこ長い処理
}

funcC() {
  // そこそこ長い処理
}

念の為のコードは書かない

この処理は無くても良いかもと思いつつ、もしかしたら想定外のケースがあるかもしれないので、念の為実装しておくという経験は皆さんあると思います。ですが、書いた本人（3ヶ月以内）は、必ずしも無くても構わないコードだということを知っていますが、読む人にはそれが分かりません。他の人が改修する際、コードの意図が分からない部分は修正せずに残すはずです。その部分があるが故に、煩雑な設計・実装になるとしても。また、念の為実装部分を修正せざるを得なくなる場合、確認に時間を浪費しやすいです。 ですので、どうしても不安で念の為コードを書く場合は、コメントで念の為コードである旨を明記しましょう。

チェックは使う人がやる

以下のように funcA で渡すパラメータを引き継いで funcC の中で使うようなケースでは、どのタイミングでパラメータをチェック（次例の場合だとnullチェック）すべきでしょうか？ できるだけ早いタイミングのチェックポイント（以降CP）1だけにしますか？CP1〜3のそれぞれでチェックしますか？

// funcAはモジュール外に公開する。
funcA(param) {
  // チェックポイント1
  funcB(param);
}

funcB(param) {
  // チェックポイント2
  funcC(param);
}

funcC(param) {
  // チェックポイント3
  param.func();
}

私はCP3だけで良いと思います。 funcC がコールされるルートは将来変わるかもしれないので、少なくともCP3ではチェックするべきです。 できるだけ早いタイミングということでCP1でもチェックする方針にすると、将来 funcA と同列の funcA' や funcA'' ができる場合にそれぞれチェックコードを書くことになります。ですので、特別な理由の無い限りCP3だけのチェックがベターだと思います。

ただし、paramを保持しておいて他の箇所からも使用するケース（モジュールの初期化オプション等）は、保存する際にチェックするべきです。複数箇所で参照するとして、使う箇所それぞれにチェック処理を書く方が煩雑になるからです。

縦横に長過ぎるコードを書かない

ひとつの関数の行数が多くなりすぎる（縦に長過ぎる）コードを書かない方が良いというのは、皆さん聞いたことがあると思うので触れませんが、縦に長くなければ良いという訳でもありません。

例えば、次のクエリストリングを生成する例ですが、横に長過ぎてとても分かりづらいです。 次例は変数名からクエリの部分だと分かるので読めますが、コードから振る舞いを読み解きつつ修正する場合ミスを招きやすいと思います。

const params = {
  param-a: paramA,
  param-b: paramB,
};
const queryString = Object.keys(params).map(key => `${key}=${(params[key as keyof typeof params] ?? '') as string}`).join('&');

過剰に汎用化しない

また上のクエリストリングの例を引き合いに出しますが、

  params = 'param-a=' + paramA;
  params += '&param-b=' + paramB;

のようにべた書きする方が分かりやすいと思います。ここら辺は意見が分かれるところだと思いますが。

パラメータ数がすごく多いとか、パラメータが動的に変わる場合は別です。さじ加減が大事だと思います。

※ よりベターなのはURL管理系のライブラリを使う事ですが、上は例題ということで。

機能単位、項目単位で分類する

ある程度ソースファイルの数が多くなってくると、目的のファイルを探しにくくなるので、分類の為のフォルダ分けが必要になります。 種類で分ける場合、機能が多くなると結局フォルダ内のファイル数が多くなり煩雑化してファイルを探しにくくなります。

view/機能A.view
view/機能B.view
process/機能A.process
process/機能B.process

以下のように機能で分ければ、機能が増えるとフォルダも増え、フォルダ内のファイルはそう多くならないのでファイルを探しやすいと思います。

funcA/機能A.view
funcA/機能A.process
funcB/機能B.view
funcB/機能B.process

似たような事で、コードをどういう分類で処理ブロックに分けるかについてです。 処理の種類で分ける場合、以下のようになります。種類で分ける場合、項目が多くなると実装位置が離れるので見通しが悪くなります。

// 代入する処理ブロック
varA = paramA
varB = paramB

// チェックする処理ブロック
if (!check(varA)) {
  varA = defA
}
if (!check(varB)) {
  varB = defB
}

処理の種類で分けるよりも以下のように項目毎に分ける方が良いと思います。 項目毎に分ければ実装位置が離れることはありません。また、項目を追加する際に修正箇所が散らばらないのでメンテしやすいです。

varA = paramA
if (!check(varA)) {
  varA = defA
}

varB = paramB
if (!check(varB)) {
  varB = defB
}

コードのコメントアウトはブロック単位にする

修正前のコードをコメントアウトして一時的に残しておくことがあると思います。

※ git等の差分を見れば分かるから即消すべきかの議論は別の機会に。

例えば以下の 処理A〜処理C の部分を 処理B だけにするケースを考えます。

  処理A
  処理B
  処理C

単純にコメントアウトすると以下になります。

  // 処理A
  処理B
  // 処理C

このケースのようにごく単純な場合はこれでも構いませんが、「処理ブロック内のあれとこれを残してそれを追加する」のような修正の場合、コードが汚く見えると思います。ですので、以下のように、修正前のブロックをまるごとコメントアウトした上で修正後のコードを書く方が良いと思います。

  // 処理A
  // 処理B
  // 処理C
  処理B

頼むから適度に空行を入れてください

関数内の各処理の前後に全く空行を入れない方がいます。空行を入れる基準を決めて統一するのが面倒くさいという気持ちは分かります。ですが、読む側からすると読みづらすぎます。空行が無いと重要な処理の行がその他処理に埋もれてしまい、見落としがちです。

func() {
  処理A-1
  処理A-2
  if (条件判定) {
    処理B-1
    処理B-2
  }
  処理大事
  処理C-1
  処理C-2
}

上だとミチっと感があるので、以下のように処理ブロックくらいの単位で空行を入れる方が読みやすくなると思います。

func() {
  処理A-1
  処理A-2

  if (条件判定) {
    処理B-1
    処理B-2
  }

  処理大事

  処理C-1
  処理C-2
}

最後に

コード品質に関する有名な書籍を紹介しておきます。

リーダブルコード

このエントリーで挙げたスタイルのいくつかはこの本にも載っています。私がたどり着いたスタイルが本に載っているのを見つけた時は、仲間がいた感じがして嬉しかったです。