はじめに
コードレビューというと、つい
・上司にコードを見てもらう
・間違いを指摘される
・自分の実装が正しいか採点される
という感覚になりがちです。
ただ、実務でコードレビューを受ける中で感じたのは、コードレビューは上司にコードの採点をしてもらう時間ではなく、よりよいコードがないかを一緒にブラッシュアップする時間だということです。
この記事では、コードレビューを受ける側として意識したいことをまとめます。
対象は以下のような方です。
・コードレビューを受ける機会がある方
・レビューで何を説明すればよいか迷う方
・「コードの説明」はしているのに、レビューで指摘が多い方
・実装力だけでなく、設計・説明力も伸ばしたい方
1. コードレビューは「採点」ではない
コードレビューを受けるとき、どうしても
・このコードで怒られないかな
・指摘が少ない方がいい
・自分のコードが正しいと認めてもらいたい
と考えてしまうことがあります。
しかし、コードレビューの目的は、実装者を評価することではありません。
本来の目的は、以下のようなものだと思います。
・要件を満たしているか確認する
・不具合につながる実装がないか確認する
・保守しやすいコードになっているか確認する
・他の実装方法の方がよくないか検討する
・チームとして安全にリリースできる状態にする
つまり、コードレビューはコードを提出して点数をつけてもらう場ではなく、チームでコードの品質を上げる場です。
指摘が入ることは、悪いことではありません。
むしろ、レビューで気づけたことで、本番障害や手戻りを防げる可能性があります。
2. 「このコードは何をしているか」だけの説明では足りない
レビュー時にやってしまいがちなのが、コードを上から順番に説明することです。
例えば、以下のような説明です。
ここで対象レコードを取得しています。
次にfor文で回しています。
条件に一致した場合はこの項目を更新しています。
最後にupdateしています。
もちろん、処理の流れを説明すること自体は悪くありません。
ただ、正直なところ、コードが何をしているかは、コードを読めばある程度わかります。
レビューで本当に確認したいのは、単なる処理内容ではなく、
・なぜこの実装にしたのか
・この実装で要件を満たせるのか
・考慮漏れはないのか
・例外ケースに対応できているのか
・将来変更が入ったときに壊れにくいか
といった部分です。
つまり、レビューで大事なのは、コードの逐語説明ではなく、要件を満たすコードが書けていることを証明することです。
3. レビューでは「要件」と「実装」をつなげて説明する
コードレビューで説明すべきなのは、コードそのものよりも、要件と実装の対応関係です。
例えば、以下のような観点です。
要件：
特定のステータス（予約済み）の予約だけを対象にする
実装：
WHERE Status__c = '予約済み' を指定して、対象外ステータスを除外している
要件：
キャンセル済みの予約は処理対象外にする
実装：
Cancel__c = false の条件を入れて、キャンセル済みデータを除外している
このように説明すると、レビュアーは
・この条件で本当に対象データを絞れているか
・別のステータスは考慮しなくていいか
・nullの場合はどうなるか
といった、より本質的な観点でレビューしやすくなります。
レビューでは、以下のように話せるとかなり良いです。
今回の要件は〇〇です。
そのため、対象データは△△に絞っています。
□□のケースは対象外なので、この条件で除外しています。
一方で、××のケースは今後追加される可能性があるため、定数化しています。
これは単なるコード説明ではなく、実装判断の説明です。
4. 「なぜそう書いたか」を説明できる状態にする
レビューでよく聞かれるのは、コードの内容そのものよりも「なぜそうしたのか」です。
例えば、以下のような質問です。
なぜこのタイミングでSOQLを取得しているのか？
なぜこの項目を条件にしているのか？
なぜMapにしているのか？
なぜこのクラスに処理を書いたのか？
なぜこのメソッド名にしたのか？
なぜこの例外処理で問題ないのか？
これらに答えられない場合、実装が悪いというより、設計判断がまだ整理できていない可能性があります。
レビュー前に、自分のコードに対して以下を確認しておくとよいです。
・この処理はどの要件に対応しているか
・この条件分岐は何を防ぐためにあるか
・このSOQLはなぜここで必要か
・このDMLは一括処理に耐えられるか
・この命名で処理内容が伝わるか
・この処理は他のクラスに書くべきではないか
・異常系やnullの考慮は足りているか
レビューで「なんとなくこう書きました」となると、レビュアーも判断しづらくなります。
逆に、「この要件を満たすために、この方法にしました」と説明できると、レビューの質が上がります。
5. 指摘されたら「修正します」で終わらせない
レビューで指摘を受けたときに、
承知しました。修正します。
だけで終わらせることがあります。
もちろん、明らかなミスであればそれでもよいです。
ただ、指摘の中には、単なる修正依頼ではなく、設計や考え方に関するものもあります。
例えば、以下のような指摘です。
この処理は共通化した方がよさそう
この条件だと別ケースを拾ってしまうかも
この命名だと意図が伝わりづらい
このクラスにこの責務を持たせるのは違和感がある
このような指摘を受けたときは、ただ修正するだけでなく、なぜその指摘が入ったのかを理解することが大事です。
・可読性の問題なのか
・責務分離の問題なのか
・ガバナ制限の問題なのか
・業務要件の考慮漏れなのか
・将来の変更に弱い設計なのか
ここを理解しておくと、次の実装で同じ指摘を減らせます。
6. レビュー前に自分でレビューしておく
コードレビューに出す前に、自分で一度レビューしておくと指摘の質が変わります。
最低限、以下のような観点を確認しておくとよいです。
・要件を満たしているか
・不要な処理が残っていないか
・命名は処理内容と合っているか
・nullの場合に落ちないか
・複数件処理に対応しているか
・ループ内SOQL/DMLがないか
・エラーメッセージは利用者に伝わる内容か
・テストデータは正常系だけでなく異常系もあるか
レビューに出す前のセルフレビューは、レビュアーのためだけではありません。
自分自身がコードの意図を整理するためにも有効です。
レビューで説明する前に、自分で
このコードは、どの要件を、どう満たしているのか？
を言語化しておくことが大事です。
7. AIに書いてもらったコードほど、自分の頭で理解してから出す
最近は、AIにコードのたたき台を書いてもらうことも増えています。
ApexやLWCでも、
・処理のひな形を作る
・SOQLの書き方を確認する
・テストクラスのパターンを出す
・リファクタ案を考える
といった使い方ができます。
AIを使うこと自体は悪いことではありません。
むしろ、うまく使えば実装スピードを上げたり、自分では思いつかなかった観点に気づけたりします。
ただし、AIに書いてもらったコードを、理解しないままレビューに出すのは危険です。
理解しないまま出すと何が起きるか
AIが生成したコードは、一見それっぽく見えても、以下のような問題を含むことがあります。
・要件と微妙にズレている
・存在しない項目やメソッドを使っている
・Salesforceのガバナ制限を考慮できていない
・複数件処理に対応できていない
・例外系やnullの考慮が足りない
・チームの設計方針と合っていない
この状態でレビューに出すと、レビュアーから質問されたときに答えられません。
なぜこの条件にしたの？
なぜこのタイミングでDMLしているの？
このnullチェックは何を想定しているの？
このクラスにこの処理を書いた理由は？
これらに答えられない場合、そのコードはまだ「自分のコード」になっていないと思います。
AIのコードは「完成品」ではなく「たたき台」として使う
AIに書いてもらったコードは、そのまま使うのではなく、必ず自分の頭で確認します。
最低限、以下は確認した方がよいです。
・今回の要件を本当に満たしているか
・対象外にすべきデータを除外できているか
・複数件処理に対応しているか
・SOQLやDMLがループ内にないか
・nullや空リストの場合に落ちないか
・既存の命名規則や設計方針に合っているか
・テストケースで正常系・異常系を確認できているか
AIが出したコードを読むときは、
「動きそうか」ではなく、
「要件を満たすことを自分で説明できるか」
を基準にするとよいです。
レビューで説明できる状態にする
AIを使ったとしても、レビューに出す時点では、自分の言葉で説明できる必要があります。
この条件は、〇〇のケースを除外するために入れています。
このMapは、ループ内SOQLを避けるために使っています。
このnullチェックは、△△が未設定のケースでエラーにしないためです。
この処理は既存の□□クラスと責務を分けるため、新規クラスにしています。
ここまで説明できれば、AIを使ったかどうかは問題ではありません。
逆に、説明できないコードは、たとえ動いていてもレビューに出すにはまだ早いです。
AIは実装を助けてくれる道具ですが、最終的にそのコードの責任を持つのは自分です。
そのため、AIに書いてもらったコードほど、自分の頭で理解してから使うことが大切です。
8. レビューは「自分のコードを守る場」ではない
レビューで指摘を受けると、つい自分のコードを守りたくなることがあります。
でも、この書き方でも動きます
一応テストでは通っています
前も同じように書いていました
もちろん、自分の実装意図を説明することは大切です。
ただし、レビューの目的は、自分のコードを正当化することではありません。
大事なのは、より良いコードにできるかどうかです。
そのため、レビューでは以下のように考えると建設的です。
・レビュアーの指摘で、より安全になるか
・読み手にとって分かりやすくなるか
・将来の変更に強くなるか
・他の実装方法の方がシンプルではないか
・チームの書き方に揃えた方がよいか
自分の実装にこだわるよりも、コード全体として良くなる選択をする方が、結果的に自分の成長にもつながります。
9. レビュー説明は「要件・判断・考慮点」で話す
レビューで説明するときは、コードを上から順番に読むよりも、以下の3点で整理すると伝わりやすくなります。
・要件：何を実現する必要があるのか
・判断：なぜこの実装にしたのか
・考慮点：対象外ケース、異常系、複数件処理などをどう扱ったのか
例えば、以下のような説明は少し弱いです。
このメソッドでは、対象の予約情報を取得して、条件に一致した場合に更新しています。
処理内容は分かりますが、要件との関係や実装判断が見えません。
より良くするなら、以下のように説明します。
今回の要件は、キャンセルされていない予約情報だけを対象に、利用日が変更された場合に後続処理用の項目を更新することです。
そのため、SOQLでは Cancel__c = false を条件に入れ、キャンセル済み予約を対象外にしています。
また、利用日が変更されていない場合は更新不要なので、旧値と新値を比較して、変更があるレコードだけ更新対象にしています。
複数件更新にも対応できるように、ループ内ではDMLを行わず、最後にまとめてupdateしています。
この説明では、以下を伝えられます。
・なぜキャンセル済みを除外しているのか
・なぜ旧値と新値を比較しているのか
・なぜ更新対象を絞っているのか
・なぜDMLを最後にまとめているのか
レビュー前に説明を整理するときは、以下の型を使うと便利です。
今回の要件は〇〇です。
そのため、対象データは△△に絞っています。
□□のケースは対象外なので、××で除外しています。
〇〇の場合のみ更新したいため、旧値と新値を比較しています。
複数件処理に対応するため、SOQL/DMLはループ外でまとめています。
レビューで必要なのは、コードの読み上げではなく、要件・判断・考慮点を自分の言葉で説明することです。
まとめ
コードレビューは、上司やレビュアーにコードを採点してもらう時間ではありません。
よりよい実装がないかを一緒に考え、コードをブラッシュアップする時間です。
そのためには、レビューを受ける側も、ただコードの流れを説明するだけでは不十分です。
大事なのは、
・何の要件に対する実装なのか
・なぜその条件にしたのか
・どのケースを対象外にしたのか
・複数件処理や例外系に対応できているか
・他の実装方法と比べて、なぜこの方法を選んだのか
・AIを使ったコードでも、自分の言葉で説明できるか
を説明できることです。
コードレビューの場で必要なのは、
「このコードはこう動きます」ではなく、
「このコードで要件を満たせます。理由はこうです」
と説明することだと思います。
レビューを怖がる必要はありません。
レビューは、自分のコードを否定される場ではなく、自分では気づけなかった観点をもらえる場です。
指摘を受けるたびに、
「なぜその指摘が入ったのか」
「次から同じ指摘を減らすには何を意識すればよいか」
を考えていくことで、実装力だけでなく、設計力や説明力も少しずつ伸びていくと思います。