Clean Code: 10 Rules I Actually Follow (And 5 I Ignore)

私は、月に23億ドルの取引を処理するフィンテック企業でシニアソフトウェアアーキテクトとして14年間他の人のコードを見つめ続けてきました。先週の火曜日、身体的にうめき声を上げたプルリクエストをレビューしました — 847行の単一関数、"data2"や"temp"のような変数名、そして文字通り"// magic happens here"と書かれたコメント。これを書いた開発者は？スタンフォードのコンピュータサイエンスの卒業生でGPAは3.9です。

その時、私は気づきました：クリーンコードを教える方法が完全に間違っていると。

皆が叔父ボブの「クリーンコード」を聖典のように引用します。彼らはSOLID原則を暗記し、タブとスペースについて議論し、顕微鏡がないと読めないほど小さな関数を書くのです。しかし、誰も教えてくれないのは、これらのルールの中には、実際にコードを悪化させているものがあるということです。

私はロバート・マーチンの仕事を批判するためにここにいるのではありません — それは基盤となる重要なものです。しかし、3,000件以上のプルリクエストをレビューし、47人の開発者を指導し、2011年から稼働しているコードベースを維持する中で、実際に重要なルールとただの開発者の演技であるルールの違いを学びました。私が言うことをお見せしましょう。

私が従うルール #1: 関数は1つのことを行うべきだ（しかしそのことはあなたが思っているより大きくなり得る）

関数の「単一責任原則」は、おそらくクリーンコードで最も誤解されているルールです。私は、"validateUserEmailFormat" のような名前で3行の長さの関数を作成し、その関数が "validateUserEmail" に呼ばれ、その結果 "validateUser" に呼ばれるという状況を目にします。それはすべて亀のように続いており、ユーザーがサインアップするときに何が起こっているのか理解するために7つのファイルを開かなければなりません。

私が実際に行うことはこうです：私は1つの意味のあるビジネス操作を達成する関数を書くのです、1つの技術的操作ではなく。私は "processPayment" という関数を書くと、それは45行の長さになるかもしれません。それは支払い方法を検証し、不正をチェックし、トランザクション記録を作成し、確認メールを送信します。これは4つの技術的操作ですが、1つのビジネス操作です：支払いの処理です。

重要なのは、各ステップがコード内で明確に区別されていることです。私は論理的なセクションを区切るために空白行を使用し、各セクションの「なぜ」を説明する簡潔なコメントを追加します。別の開発者が "processPayment" を読むと、彼らは12の異なるファイルの間を飛び回ることなく、全体の支払いフローを理解できます。

私はこれを一度測定しました。私たちの古いコードベースでは、私たちは「関数は小さくなければならない」というルールを厳守していたため、平均的な開発者は単一のユーザーフローを理解するために8.3ファイルを開かなければなりませんでした。意味のある操作関数にリファクタリングした後、その数は2.1ファイルに減りました。コードレビューの時間は34%減少しました。バグ修正の時間は28%減少しました。

ルールは「関数を小さく作る」ではありません。ルールは「関数を理解可能にする」です。時には10行を意味することもありますし、時には50行を意味することもあります。決して意味しないのは、開発者にあなたの全コードベースを調べさせて、ボタンクリックがどのように機能しているかを見つけ出すことです。

私が従うルール #2: 名前は意図を示すべきだ（たとえそれが長くても）

かつて、ある開発者が変数名は15文字を超えてはいけないと主張していたことがあります。なぜなら「それがコードを読みづらくするから」です。彼はこんなコードを書きました："const usrPmtMthd = getUserPaymentMethod();" 私はキーボードを窓から投げ捨てたくなりました。

"最良のコードは最も巧妙なものではなく、次の開発者がシステムダウンで顧客が電話をかけているときに理解できるコードです。"

私のルールはこうです：変数の名前を一度読んでその内容が理解できない場合、その名前は間違っています。私はそれが40文字であろうと気にしません。「pmtMthd」が何を意味するのかを理解するのに3分かかるよりも、「userSelectedPaymentMethodFromDropdown」を読むほうがいいです。

私たちの支払い処理システムでは、「transactionRequiresAdditionalFraudVerificationBasedOnUserHistory」という変数があります。これは71文字です。また、非常に明確です。その変数がif文の中にあるとき、何がチェックされているのか、なぜなのかが正確にわかります。コメントは必要ありません。精神的な翻訳も必要ありません。

私がいつも耳にする反論は「しかし長い名前は行を長くする！」というものです。確かに、1985年の80列の端末でコードを書いているふりをしているのであれば。今は27インチのモニターがあります。それを使いましょう。私の行長制限は120文字に設定しており、読みやすさの問題は一度もありませんでした。

私が使用するテストはこうです：ジュニア開発者があなたの変数名を読んで、その内容、型、おおよその使い方がすぐにわかるなら、それは正しく名付けられています。もし彼らが宣言された位置を確認したり、型定義を見に上にスクロールしなければならないなら、あなたは失敗したのです。

私はこれがコードレビューの質を劇的に向上させるのを見てきました。名前が明確であれば、レビュアーは「これが何をするのか？」を尋ねるのにかける時間が少なくなり、「これは正しいアプローチか？」を尋ねるのにもっと時間をかけることができます。そこに本当の価値があります。

私が従うルール #3: コメントはなぜそうするのかを説明するべきだ、何をするのかではなく（しかしあなたが思っているより多く使用すべきだ）

クリーンコードの純粋主義者たちは「良いコードはコメントが必要ない」と言います。彼らは半分正しいです。良いコードは何をするのかを説明するコメントが必要ありません。しかし、それは絶対になぜそのコードが存在するのかを説明するコメントが必要です。

先月、私のコードベースに奇妙な回避策がある関数を見つけました：ウェブフックを処理する前に50ミリ秒の遅延を追加していました。コメントはありません。説明もありません。ただ、"await sleep(50);" が地雷のように座っているのです。私はそれがバグなのか意図的なのかを理解するのに2時間かかりました。その後、これは、3日間の生産環境でのデバッグの後に発見した、サードパーティAPIのレースコンディションに対する回避策でした。

今、そのコードにはコメントがあります："// 50msの遅延が必要：Stripeのウェブフックは彼らのAPIが課金のステータスを反映する前に到着する可能性があります。2023年8月15日のインシデント#2847で発見されました。Stripeが最終的一貫性の問題を修正した後にこれを削除してください（チケット#45892）。"

これは良いコメントです。それは、コードが存在する理由を説明し、それを引き起こしたインシデントを参照し、将来的にそれを削除するための道筋を提供しています。そのコメントがなければ、次の開発者はそれが間違いだと思って削除したでしょう。

私は、次の3つの状況でコメントを自由に追加します：依存関係のバグを回避する際、明白でないビジネスルールを実装する際、及びコードの可読性を下げる形でパフォーマンスを最適化する際。その場合、コメントはコード自体では見えないコンテキストを説明します。