🐟

僕の思う綺麗なコードを言語化してみる

2022/01/25に公開

ポエム

idea

はじめに

大学時代は, 美しいコードとは何かを常々説かれてきました。そんな感じなので, 僕自身も美しいコードを見るのは好きですし, 書くときは美しさを追求します。

というところで, 最近の会社生活では, 直感的に「このコードは綺麗だなー」という気持ちにはなるのですが, 他者に説明するとなると言葉がなかなかついてこないです。

せっかくなので今回はまったりと言語化してみようかなと思います。
なお, 美しいコードのなんたるかについては論争激しいので, あくまで本記事は一個人の見解となりますのでご了承ください。
また, 本記事は, 主観直感を言語に落とそうとしているだけなので, 理論に紐付けなどは実施してませんのでご留意ください。

綺麗なコードとは

美しいコードと書き続けると仰々しい気がするので以降は綺麗なコードと書きます。

「おっ, これは綺麗だなー」と思うコードって何でしょうね。
個人的には, ↓ の内容を踏まえていると綺麗だなーと思います。

読みやすい
テストしやすい
責任範囲がわかりやすい
追加の実装しやすい

当初の「直感ばかりだから言語化する！」という予定に反して, 完全に主観だよりの条件を書いちゃいました。これだとわからないのでさらにこれらの要素に対して詳しく書きます。

「読みやすい」とは

コードの「読みやすさ」は, どんな点にあるのかなと考えたとき, 以下が気にされてると助かったなー。と思いました。（まだまだ色々あると思いますが一旦このあたり）

名前がわかりやすく, 一貫性がある（規則性が見出せる）
処理のグループが見える（グループごとに空行差し込み, 関数分離など）
処理の流れがスッと読める（1 分程度で見通せるイメージ）
個々の関数・コンポーネント・ファイル内のコード量が多くない
インデントやスペース等, 適切にフォーマットされていて, 一貫性がある

最後のフォーマットに関しては, Formatter がいい感じにしてくれるので, 気にしなくても一貫性が取れていることが多いかなと思います。（Formatter に強制される時代ですが, 自論は持っておくとなんとなく楽しいです。）

名前がわかりやすく, 一貫性がある（規則性が見出せる）

名前がわかりやすい は例えば

const isActive = hoge.isActive();

という記述があったとき, isActive 関数の内容を見なくても, この行のみで何しているかが読み取れそうです。isXXXX の形なので boolean が返却されるのかなーと思いますし, hoge が Active かどうかを hoge の内部値で確認しているのかなーと思います。
一方で, isActive 関数の中身が, hoge が持つ Active Object を返却する といった処理になっているとちょっとわかりづらい印象を受け, isActive 関数の内容を読み込む必要がでてきそうです。
これは, is や exist, on, append, get など, 一般的に関数でよく使われる接頭語には, よく使われる構成があり, これに反した実装を書くと読みづらくなってしまうという感じです。

別の例として, 以下のようなものを考えます。

const message = hoge.getMessage();
const fugaMessage = fuga.getMessage();
// message や fugaMessage に関する後続処理

これは, message に当たる変数が複数必要だったときに, 最初のものは素直な名前 message にして, その後の重複する名称にはとりあえず Prefix つけて fugaMessage にした状態です。
このとき, hoge.getMessage() と fuga.getMessage() はただ記載順序がこの順番だっただけで, 同じような message 用のコードです。どちらかが真の message だという世界線でもないです。にもかかわらず命名規則が違うと, 後続処理内で message 変数が hoge と fuga のどちらを示すか直感でわからなくなります。hoge と fuga どちらがより message 感強いか？わからんねって話です。宣言場所に戻って確認させることになるので, 同じようなものには同じような規則で名前をつけたいですね。

そのほか, レビュー時に気にしてる点とかだと, 同じ PR / ファイル内で命名のポリシーを見つけられるか（複数を意味する変数を意味もなく XXXXList, XXXXCollection, XXXXs（複数形） とバラバラにしたりしてないか）とかですね。

処理のグループ・流れが見える

処理のグループが見える（グループごとに空行差し込み, 関数分離など）
処理の流れがスッと読める（1 分程度で見通せるイメージ）

上記は, イメージとしては, ディレクトリ分けとか目次みたいな感じですかね。無秩序に 1 フォルダに大量のファイルが入っているより, 種類とか役割ごとにフォルダで固まってた方が良いじゃないですか。目次とかも, 近しい話がまとめられて, なおかつわかりやすいように順序立てられてると頭に入りやすいです。実装もそんな感じで, ちょっと処理や役割ごとに分けられていて順序もわかりやすいと読みやすいはず。です。

かなり雑なサンプルを出すと, x → y → z の順に完了することが必要な処理（x, y, z の内部処理間に依存はなし）の場合, 無秩序なのは以下な書き方で,

const sample() = () => {
    x1();
    x2();
    y1();
    y2();
    x3();
    y3();
    z1();
    z2();
    y4();
    y5();
    y6();
    y7();
}

これはとりあえず, わかりやすように固めてつつ空行でグループが見えるようにしてあげて

const sample() = () => {
    x1();
    x2();
    x3();

    y1();
    y2();
    y3();
    y4();
    y5();
    y6();
    y7();

    z1();
    z2();
}

x → y → z の流れを見せたいのに, y の占める処理が多すぎて y から圧力を感じるので, どこかに関数で追い出してあげる。

const sample() = () => {
    x1();
    x2();
    x3();

    y(); // y1 ~ y7 の内容を別関数へ切り出し

    z1();
    z2();
}

これなら, 一目で x → y → z の流れが掴めそうです。まぁ現実のコードはより複雑なのですが, 基本の概念は同じです。僕が好まないのは, 全体の流れを読むために, めっちゃスクロールしなければならなかったり, x, y, z の中, y だけ異常にインデント深くなってたりとかしてると読みづらいかなと思います。

個々の関数・コンポーネント・ファイル内のコード量が多くない

これは, 10 行目に呼ばれている関数がそのコードファイル内の 800 行目に実装されてるとか, 一つの関数実装が 300 行続くとかをやめたいということです。

前者の理由は, IDE とかで関数の実装場所に移動とかの機能があると思うんですが, これ使って 10 行目から 800 行目に飛ばされると悲しくなるからです。（もはや感情的ですみません）10 行目に戻るのも面倒だし, 10 ~ 800 行目の間に何の実装があるのか読み取るのも面倒だし, 以降もいろいろな行に飛ばされ続けるのかと思うと, 憂鬱なので膨大な行数を生成するのはちょっと好きじゃないです。

後者の理由は, 300 行続く膨大な一続きの処理を読み切ったとしても, 再度, 「この処理はどの辺かなー」と思った時に 300 行漁り直しになるからです。できるだけ処理分けできる箇所は関数に切り出してあげたいです。全探索したくないという話です。

「テストしやすい」とは

「テストのしやすさ」は, とりあえずテスト書いてみたら多分わかってくる概念だと思います。
最近テストが書きづらかったコードで, 気にしたいなーと思ったのは以下とかです。

個々の関数・コンポーネントのコード量が多くない
個々の関数・コンポーネントの役割が明確（できるだけ役割は単一がいい）
処理の分岐が多くない
無意味な重複処理がない
（副作用と参照透過性のはなし：割愛）

個々の関数・コンポーネントのコード量が多くない

読みやすさのときにも触れた項目ですね。テストってかなり実装量が多くなるので, テスト対象のコードが長ければ長いほど, テストコードもぐんぐん育ちます。条件分岐網羅とかしていくと, めちゃめちゃ育ちます。できるだけテスト対象のコードはうまく関数分けしたりコンポーネント分けしたいですね。

処理の分岐が多くない

これは前述したコード量のあたりでも触れましたが, 分岐が増えると網羅するためのテストコードが増えます。分岐がネストしてるとさらにきついです（倍々になるので）。せめて一テスト対象内の分岐のネストはある程度排除したいです。

無意味な重複処理がない

これは, テストができない箇所ができたりする弊害も出てくるので, 重複処理は排除したいという感じです。（単純に無意味な重複は削除していきたいですが）

例えば, 以下のコードの場合, value の null, empty のチェックが重複してます。

const sample = (value) => {
  if (!value) {
    return "Empty (if)";
  }
  return value ? "Not Empty" : "Empty (return)";
};

このとき, テストを書くと value(null) や value('') は, "Empty (if)" が返却されます。
value('ok') は, "Not Empty" が返却されます。では, "Empty (return)" を返すテストはどう書くのか？というときに困ります。value の null, empty チェックはすでに if で済んでいるため, value? の時点で false となれないからです。

ということで, 以下な感じに重複を排除してしまってテストしやすくします。

const sample = (value) => {
  if (!value) {
    return "Empty (if)";
  }
  return "Not Empty";
};

「責任範囲がわかりやすい」「追加の実装しやすい」

このあたりは長くなりそうなので, 追々, 別記事などで触れようかなと思います。軽くだけかきました。

コードファイル・関数・コンポーネントはどのような責任を持っているのか, 責任範囲はどこまでか, をしっかり考えて理論を持って実装することが大事かなと思います。

例えば, メールを送る処理を考えた時に, 「必要なデータを集約して整形」→「整形済データをメールテンプレートに差込み」→「メールサーバに送る（エラー処理）」という感じで責任を分割して関数分けしたり, コードファイルを分けたりします。

将来拡張しようとしたときや類似機能を実装するときに, 役割ごとに分割されていると, どこに書く迷うことが少なくなってきます。

あとがき

なかなか言語化って難しいなと思いました。後半の方, 力尽きて流してしまったのでまた今度追記していきます。