私はもうしばらく、正確には20年以上にわたって他人のコードをレビューしてきました。現在は時間の50〜70%ほどを、何らかの形でコードレビューに費やしています。システム設計と並んで、それが私の仕事なのです。

その間に、効果的なコードレビューの方法について、いくつか学ぶことがありました。今ではレビューで注目するポイントも、始めた頃とは違っています。

全体像を考える

良くないレビューは視野が狭くなりがちです。保守性や拡張性ではなく、構文やスタイル、些細な問題ばかりに目が向いてしまいます。

良いレビューでは、変更点そのものだけでなく、その変更がどんな問題を解決するのか、将来どんな問題が起こり得るか、そしてシステム全体の設計にどう収まるかまで見渡します。

私はあえて変更されなかった行に目を向けるようにしています。そこにこそ本当の物語が隠れていることが多いからです。

たとえば、関連するコードやドキュメントの更新を忘れてしまうことはよくあります。そうした漏れはバグや混乱、破壊的な変更、セキュリティ上の問題につながりかねません。

徹底的に、新しいコードのすべての呼び出し元を確認しましょう。正しく更新されているでしょうか？テストは今も適切な内容を検証しているでしょうか？変更は適切な場所で行われているでしょうか？

これらの問いは、変更そのものよりもシステム設計に関わるものです。良くない変更を受け入れてしまうとシステムは脆くなっていくので、全体像を見失わないようにしましょう。

コードは単独で書かれるものではありません。より経験豊富な開発者の役割は、運用上の摩擦を減らし、プロジェクトのリスク管理を担うことです。ドキュメントもテストもデータ型も、コードそのものと同じくらい重要です。

コードが進化していく中で、常により良い抽象化の可能性に目を光らせておきましょう。

命名がすべて

コードレビューでは、良い名付けを考えることに多くの時間を費やしています。

名付けは難しいものですが、だからこそ正しく行うことが重要です。多くの場合、それはコードレビューで最も重要な部分でもあります。

同時に最も主観的な部分でもあるため、単なる重箱の隅をつつく指摘なのか、重要な命名の判断なのかを見分けるのが難しく、骨の折れる作業でもあります。

名前は概念をカプセル化し、コードにおける「構成要素」として機能します。悪い名前は、より根深い問題を示すコードスメルです。認知的負荷を一桁以上も増大させてしまいます。

たとえば、ゲームでプレイヤーのステータスを表す次のようなstructがあるとします。

struct Player {
    username: String,
    score: i32,
    level: i32,
}

よく見かけるのが、こんなコードです。

// Bad: using temporary/arbitrary names creates confusion
fn update_player_stats(player: Player, bonus_points: i32, level_up: bool) -> Player {
    let usr = player.username.trim().to_lowercase();
    let updated_score = player.score + bonus_points;
    let l = if level_up { player.level + 1 } else { player.level };
    let l2 = if l > 100 { 100 } else { l };

    Player {
        username: usr,
        score: updated_score, 
        level: l2,
    }
}

このコードは読みにくく、理解しづらいものです。usrやupdated_score、l2が何を指すのか、目的が明確に伝わってきません。認知的負荷が高まり、ロジックを追うのが難しくなります。

だからこそ私は、たとえ細かすぎると思われても、変数に最もふさわしい名前を常に考えるようにしています。

// Good: meaningful names that describe the transformation at each step
fn update_player_stats(player: Player, bonus_points: i32, level_up: bool) -> Player {
    // Each variable name describes what the value represents
    let username = player.username.trim().to_lowercase();
    let score = player.score + bonus_points;

    // Use shadowed variables to clarify intent
    let level = if level_up { player.level + 1 } else { player.level };
    let level = if level > 100 { 100 } else { level };

    // If done correctly, the final variable names
    // often match the struct's field names
    Player {
        username,
        score,
        level,
    }
}

良い名前は、値の宣言場所と使用場所が遠く離れ、多くの開発者が問題領域について共通理解を持つ必要がある大規模なコードベースでは、さらに重要になります。

「ノー」と言うことを恐れない

私も変更を却下しなければならないことが頻繁にありますが、決して簡単なことではありません。相手は多くの労力をかけており、その成果を受け入れてほしいと思っているのですから。

決定を曖昧にしたり、ただ優しくしようとしたりするのは避けましょう。客観的に、理由を説明し、より良い代替案を提示してください。くよくよせず、次のステップに目を向けることが大切です。

正しくなく、後々問題を引き起こすようなものを無理に受け入れるより、ノーと言う方がずっと良いのです。一度前例を作ってしまうと、後から変更を断るのはさらに難しくなります。

それこそがレビュープロセスの目的です。コードが必ず受け入れられる保証はないのです。

オープンソースでは、基準を満たさないコードを寄せる人も多くいます。誰かが「ノー」と言わなければならず、それは非常に不人気な役割です（オープンソースのメンテナーなら誰に聞いてもそう答えるでしょう）。しかし、優れたプロジェクトには門番が必要です。そうしなければ質の低いコードが蓄積し、やがて保守不可能なプロジェクトになってしまうからです。

時には「とりあえずマージして、あとで直そう」と言う人もいます。私はそれは危険な坂道だと思っています。技術的負債や後々の余計な作業につながりかねません。自分の立場を貫くのは難しいことですが、重要です。何かおかしいと感じたら、声を上げましょう。

辛くなったときは、あなたが拒絶しているのは人ではなくコードなのだと思い出してください。相手の努力に感謝していること、そして改善を手伝いたいと思っていることを伝えましょう。

レビューで何に注目すべきか直感が磨かれてきたとしても、やはり事実で裏付けることが大切です。同じ理由で何度も「ノー」と言っていることに気づいたら、チーム向けのスタイルガイドやガイドラインを作ることを検討してみてください。

礼儀正しく、しかし決断は明確に。所詮はコードなのですから。

コードレビューはコミュニケーション

コードレビューは単にコードだけの問題ではありません。人も大切なのです。同僚と良い関係を築くことが重要です。

私は可能であれば、最初の数回のレビューはペアプログラミングの形で一緒に行うようにしています。

そうすることで、お互いのコミュニケーションスタイルから学ぶことができます。信頼を築き、相手を知るのにも効果的です。もしコミュニケーションのすれ違いや誤解に気づいたら、後でまた同じように一緒にやってみると良いでしょう。

複数回に分けてレビューする

「このPRをさっと見てもらえますか？今日中にマージしたいんです」。コードレビューは一度で終わるものだと思われがちですが、そうではありません。コードレビューは反復的なプロセスです。コードを正しく仕上げるためには、複数回のやり取りがあって当然だと考えましょう。

私は1回目のイテレーションでは、全体像と全体的な設計に焦点を当てます。それが終わってから、細部に入っていきます。

目標はできるだけ早くマージすることではなく、質の高いコードを受け入れることであるべきです。そうでなければ、そもそもコードレビューをする意味がありません。これは意識を切り替えるべき重要なポイントです。

レビューは決して欠点を指摘することだけが目的ではありません。チーム内でコードに対する共通理解を作ることでもあります。私は他の人のコードをレビューすることで、より良いコードの書き方について最も多くを学んできました。優れたエンジニアから自分のコードに素晴らしいフィードバックをもらったこともあります。

そうしたかけがえのない「なるほど」という瞬間が、開発者としての成長を助けてくれます。専門家たちが貴重な時間を使って私のコードをレビューしてくれ、そこから多くのことを学びました。誰もがキャリアの中で一度はそうした経験をすべきだと思います。

嫌な人にならない

時には書き手と意見が合わないこともあります。敬意を持ち、建設的であることが重要です。人格攻撃や見下した言葉は避けましょう。「これは間違っている」と言うのではなく、「私はこうするだろう」と言いましょう。相手が躊躇しているようなら、相手の考えを理解するためにいくつか質問してみてください。

• 「この方法だと既存のワークフローが壊れたりしませんか？」
• 「他にどんな代替案を検討しましたか？」
• 「この関数を空の配列で呼び出すとどうなりますか？」
• 「この値を設定しなかった場合、ユーザーにはどんなエラーメッセージが表示されますか？」

このような「ソクラテス式問答」¹は、書き手が自分の決定について考える助けとなり、より良い設計につながることがあります。

人々はあなたのフィードバックを喜んで受け取れるべきです。そうでないなら、自分のレビュースタイルを見直してみましょう。自分自身がもらって嬉しいと思うコメントだけを残すようにしてください。

私は時々「いいですね」や「素晴らしいアイデアだと思います」といった肯定的なコメントを添えるようにしています。書き手のモチベーションを保ち、その仕事に感謝していることを示すことは、大きな効果があります。

可能であれば、コードを実際に動かしてみる

長時間コードを見続けると、些細なディテールを見逃しがちです。手元にコピーがあって、いろいろ試せる状態にしておくと大いに役立ちます。

可能であれば、コードやテスト、リンターを実際に実行してみます。ブランチをチェックアウトし、あちこち動かしたり、あえて壊してみたりして、どのように動くのかを理解しようとすることが、私のレビュープロセスの一部です。

UIの変更やエラーメッセージのようなユーザーに直接影響する変更は、実際にコードを動かして壊そうとしてみることで、問題に気づきやすくなります。

その後は変更を元に戻し、必要に応じて気づいたことをコメントにまとめます。このアプローチによって、より深い理解が得られるのです。

自分の対応可能時間を正直に伝える

コードレビューは開発プロセスのボトルネックになりがちです。完全に自動化することはできず、コードを見てフィードバックを提供する人間が介在する必要があるからです。

しかし、同僚がコードをレビューしてくれるのをただ待つだけでは、フラストレーションにつながります。そうならないようにしましょう。

時にはコードをレビューする時間がないこともありますが、それは仕方のないことです。妥当な時間内にレビューできない場合は、その旨を書き手に伝えましょう。

私自身まだ改善の途中ですが、自分の対応可能状況についてより主体的に伝え、明確な期待値を設定するように心がけています。

学び続けることをやめない

コードレビューは、私にとって新しいことを学ぶための最も好きな方法です。新しいテクニックやパターン、新しいライブラリ、そして何よりも他の人がどのように問題にアプローチするかを学ぶことができます。

私はレビューごとに一つ新しいことを学ぼうとしています。それがチーム全体の改善や成長につながるのであれば、決して無駄な時間ではありません。

細かすぎる指摘はしない

フォーマッターが存在するのには理由があります。空白やフォーマットはツールに任せましょう。本当に重要な問題のためにエネルギーを使いましょう。

ロジック、設計、保守性、正確性に焦点を当ててください。コードの品質に影響しない主観的な好みは避けましょう。

自問してみてください。「これは機能に影響するか、あるいは将来の開発者を混乱させるだろうか？」そうでなければ、見逃して構いません。

方法ではなく理由に焦点を当てる

コードをレビューする際は、変更の背後にある理由に焦点を当てましょう。理由もなく欠点を指摘するよりも、はるかにうまくいく可能性が高まります。

次の2つのコードレビューコメントを比べてみてください。1つ目は役に立たず、突き放すようなものです。

2つ目は代替案を提示し、ドキュメントへのリンクを添え、なぜその変更が将来的に問題を引き起こす可能性があるのかを説明しています。

あなたなら、どちらのコメントを受け取りたいでしょうか？

これにはより多くの時間と労力がかかることは承知していますが、それだけの価値はあります！多くの場合、書き手は感謝し、将来同じ間違いを避けるようになります。役立つレビューには、時間をかけて積み重なる複利効果があるのです。

愚かな質問を恐れない

推測するより聞いた方が良いのです。理解できないことがあれば、書き手に説明を求めましょう。あなただけが理解できていないわけではない可能性が高いのです。

多くの場合、書き手は喜んでその理由を説明してくれます。そこからコードやシステム全体への理解が深まります。また、書き手自身が違う視点から物事を見るきっかけにもなります。もしかすると自分の前提が間違っていたことや、システムが自明ではないことに気づくかもしれません。あるいはドキュメントが不足しているのかもしれません。

優れた質問をすることは、超能力なのです。

自分のレビュースタイルについてフィードバックを求める

時々、書き手に自分のフィードバックに対するフィードバックを求めてみましょう。

• 厳しすぎたり、細かすぎたり、遅すぎたり、雑だったりしませんでしたか？
• 適切な点を指摘できていましたか？
• フィードバックは役に立ちましたか？
• 改善のための提案はありますか？

要するに、自分のレビュープロセスをレビューしてもらうのです。不思議な感じですが。

コードのレビュー方法を学ぶことは、絶え間ない練習と改善を必要とするスキルです。あなた自身のスタイルが見つかることを願っています。