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

時間をかけて、効果的なコードレビューのやり方をいくつか学びました。始めた頃とは、今は注目する点が違います。

全体像を考える

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

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

私は、変更されていない行を見るのが好きです。そこに本当の事情が表れていることがよくあります。

たとえば、関連するコードベースの箇所やドキュメントの更新を忘れることはよくあります。そうすると、バグ、混乱、破壊的変更、セキュリティ上の問題につながる可能性があります。

徹底的に確認し、新しいコードのすべての呼び出し箇所を見てください。正しく更新されていますか？ テストはまだ正しいものをテストしていますか？ 変更は適切な場所にありますか？

これらの質問は、変更そのものよりもシステム設計に関わるものです。悪い変更を受け入れるとシステムは脆くなるので、大きな視点をおろそかにしないでください。

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

コードが進化する中で、よりよい抽象化がないか常に目を配ってください。

命名がすべて

コードレビューでは、よい名前について考えることにかなりの時間を使います。

物に名前を付けるのは難しいからこそ、正しく行うことが重要です。多くの場合、コードレビューで最も重要な部分です。

一方で、最も主観的な部分でもあります。そのため、単なる揚げ足取りと重要な命名判断を区別しにくく、面倒でもあります。

名前は概念を包み込み、コードにおける「構成要素」になります。悪い名前は、根深い問題を示唆するコードスメルです。認知的な負荷を一桁以上増やします。

たとえば、ゲーム内のプレイヤーのステータスを表す構造体があるとします。

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をさっと見てもらえますか？ 今日中にマージしたいんです」。コードレビューは一度きりのものだという期待があることがよくあります。しかし、そうではありません。コードレビューは反復的なプロセスです。コードを正しく仕上げるには、複数回のレビューを前提にすべきです。

最初のレビューでは、大局と全体の設計に注目します。それが終わってから、細部に入ります。

目標はできるだけ早くマージすることではなく、高品質なコードを受け入れることです。そうでなければ、そもそもコードレビューをする意味があるでしょうか？ これは切り替えるべき重要な考え方です。

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

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

嫌なやつにならない

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

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

こうした「ソクラテス式の質問」¹は、作者が自分の判断について考える助けになり、よりよい設計につながることがあります。

人には、あなたからフィードバックを受け取ることを楽しんでほしいものです。そうでないなら、自分のレビューのスタイルを見直してください。自分が受け取ってうれしいと思えるコメントだけを加えましょう。

ときどき、「これ、いいですね」や「これは素晴らしいアイデアです」といった肯定的なコメントを加えるようにしています。作者のモチベーションを保ち、その仕事を評価していると示すことには大きな効果があります。

可能ならコードを動かしてみる

コードを長時間見ていると、微妙な細部を見落としがちです。手元に試せるコードのコピーがあると、とても助かります。

できるなら、コード、テスト、リンターを実行するようにしています。ブランチをチェックアウトし、あちこち動かし、壊してみて、仕組みを理解しようとすることも、私のレビュー手順の一部です。

UIの変更やエラーメッセージといったユーザー向けの変更は、コードを動かして壊してみると、問題を見つけやすいことがよくあります。

その後、変更を元に戻し、必要なら見つけたことをコメントに書きます。このアプローチから、より深い理解が得られることがあります。

対応可能な時間をあらかじめ伝える

コードレビューは完全には自動化できないため、開発プロセスのボトルネックになりがちです。人がループに入り、コードを見てフィードバックを提供しなければならないからです。

しかし、同僚のレビューを待たされると、フラストレーションにつながります。そんな人にならないようにしましょう。

コードをレビューする時間がないこともあります。それは問題ありません。妥当な時間内にレビューできないなら、作者に知らせてください。

私自身まだ改善中ですが、対応可能な時間についてより積極的に伝え、明確な期待値を設定するようにしています。

学び続ける

コードレビューは、新しいことを学ぶ私のお気に入りの方法です。新しい技法、パターン、新しいライブラリを学びますが、最も重要なのは、他の人が問題にどう取り組むかを学べることです。

レビューのたびに一つ、新しいことを学ぶようにしています。チーム全体の改善と成長につながるなら、無駄な時間ではありません。

細かすぎる指摘をしない

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

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

自問してください。これは機能に影響しますか？ あるいは、将来の開発者を混乱させますか？ そうでなければ、手放しましょう。

「どうやって」ではなく「なぜ」に注目する

コードをレビューするときは、変更の背景にある理由に注目してください。理由なしに欠点を指摘するよりも、はるかに成功する可能性が高くなります。

次の二つのコードレビューコメントを考えてみてください。最初のものは役に立たず、突き放したものです。

二つ目は代替案を示し、ドキュメントにリンクし、その変更が後々問題につながる理由を説明しています。

受け取りたいのはどちらでしょうか？

こちらのほうが時間と労力がかかることはわかっていますが、その価値はあります。多くの場合、作者は感謝し、今後同じ間違いを避けられるようになります。役に立つレビューは、時間とともに複利のような効果を生みます。

愚かな質問をすることを恐れない

推測するより、質問するほうがよいです。わからないことがあれば、作者に説明を求めてください。理解できていないのは、あなただけではない可能性が高いです。

多くの場合、作者は自分の考えを喜んで説明してくれます。その結果、コードとシステム全体への理解が深まることがあります。また、作者が別の視点から物事を見る助けにもなります。自分の前提が間違っていた、あるいはシステムが自明ではなかったと学ぶかもしれません。ドキュメントが足りないのかもしれません。

優れた質問をする力は、強力な武器です。

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

ときどき、作者にあなたのフィードバックへのフィードバックを求めてください。

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

要するに、自分のレビュープロセスをレビューしてもらうのです。へへ。

コードレビューを学ぶことは、継続的な練習と改善が必要なスキルです。自分なりのスタイルを見つけてください。