読みやすいコード、メンテナンスしやすいコード

自分が個人的に読みやすい、メンテナンスしやすいと思うソースコードの特徴を書いてみようと思います。個人の好みも含みます。
言語やアーキテクチャ、プロジェクトにかかわらず、コードレビューの時にはこのような観点でレビューしてます。

モジュール名・クラス名・関数名・変数名からその責務が明確である

基本的に、一つの責務を持つ
- GOOD: getBook(id: string), updateBook(book: Book)
- BAD: getAndUpdateBook(bookId: string)
その責務がわかるように命名する
- 英語として成り立ってる（日本語に直訳して意味が通じる）
- 第三者が見て別の意味に捉えようがない
ただし、具体的に説明しすぎず、適宜抽象化する
- 具体的にしすぎると、処理をちょっと変えるたびに関数名も変更が必要になってしまうし、関数名が長くなりすぎる傾向がある
- 例: 本リストを filter する関数のケース
  - GOOD: filterBooks(id: string, name: string, index: string)
  - BAD: filterBooksByIdAndNameAndIndex(id: string, name: string, index: string)
    - filter の条件などいつ変わるかも分からないので、本リストをフィルターすることだけを書けば良さそう
どうしても二つ以上の責務を持たせたい場合は、関数名で表現する
- GOOD: getBookAndRefreshCache(bookId: string)
- BAD: getBook（キャッシュが更新されることが分からない）

関数の処理内容は引数のみに依存させ、かつ、副作用がない

関数の外側世界（環境変数、グローバル変数、外側のスコープの変数、ファイルなど）は利用しない
引数、および、関数の外側世界を変更しない
環境変数利用とかDB保存とかキャッシュ保存など必要なケースはしょうがないが、その場合も外側世界に依存する箇所（クラス、関数）を最小化する
- 例: config クラスで環境変数を読み込んで、その他は config オブジェクトを引数に取るようにする
- 例: DB 保存する処理は repository 実装クラスに任せ、その他は repository インターフェースを呼び出す

ソースコードを全て読まなくても、処理内容を把握できるようにする

呼び出し元側で処理内容の全体像を把握できる
- 一つの関数が一つの責務を持つようにしておけば、自然にそうなる
  ステップインしなくても関数が何をやってるか分かる
- 基本は以下のような感じ
  呼び出し元側で、関数名を順番に読めば何をやってるか分かる
  以下の例では、呼び出し元（doHoge）だけで「最初に入力チェックして、次に何か値を変更して、保存する」という全体像を把握できる。全体像が分かれば、自分が注目したい箇所だけ細かく見れば良い
```
function doHoge(hoge: string): void {
  if (!isValid(hoge)) return;
  const changedHoge = change(hoge);
  register(changedHoge);
}

function isValid(hoge: string): boolean {
  // do something
}
function change(hoge: string): string {
  // do something
}
function register(hoge: string): void {
  // do something
}
```

アーリーリターン
その時点で処理が確定するので、先読みしなくて良くなる。また、階層も浅くなるので読みやすい

// GOOD
function doHoge(hoge: string | undefined): void {
  if (hoge === undefined) return;
  // do something
}

// GOOD
function doHoge(hoge: string | undefined): void {
  if (hoge === undefined) throw new Error('hoge is undefined.');
  // do something
}

// BAD
function doHoge(hoge: string | undefined): void {
  if (hoge !== undefined) {
    // do something
  } else {
    return;
  }
}

各レイヤーの役割が決まっている
- クリーンアーキテクチャでもレイヤードアーキテクチャでもMVCでもいいけど、そのレイヤでやることが明確である
- どこで何が実装されているか当たりがつくようになる

関数／変数のスコープはできるかぎり小さくする、かつ、イミュータブルにする

*　スコープが小さければ、利用場所が限定され、メンテナンス時に見る範囲が狭くなる

ミュータブルな変数はどこで書き変わるか分からないので、コードを全部追わなければいけなくなる

同じコードが複数出現しない（DRY: Don't Repeat Yourself）

基本は2回、少なくとも3回、同じコードが出現したら共通化する
DRY じゃないソースコードは修正漏れが頻繁に発生する

プロジェクトで書き方を統一する

プロジェクト全体で統一が難しくても、少なくともファイル内では書き方を統一したい
レガシーなソースコードと新しいソースコードが混在したコードは、レガシーだが統一されているコードより見にくいケースが多い

Tell-Don't-Askの法則（命じろ、尋ねるなの法則）

関数の処理条件を、呼び出し側で知っている状態は明らかに密結合。関数の仕様が変わると呼び出し元も変更が必要になる。その関数の中でよしなに処理するようにする。

// GOOD
function doHoge(input: number): void {
  if (input < 10) return;
  // do something
}
const input = 99;
doHoge(input);

// BAD
function doHoge(hoge: number): void {
  // do something
}
const input = 99;
if (input >=10) {
  doHoge(input);
}

コーディングスタイルは適切に

脳のリソースをここにさきたくない
linter / formatter を導入して、機械に任せる
- こちらに設定方針、設定方法をまとめたので、興味があれば。

テストは書く

テストが充実していると、リファクタリングのリスクが下がるので、リファクタリングしてコードを改善しやすい。