3秒でわかる
書いたコードを別の人が読んで指摘する工程。バグを本番前に見つけ、書き方の基準と仕様の知識をチームに広げるために行います。
30秒図解
もう少し詳しく
どういうものか
コードレビューは、変更されたコードを作者以外の人が読み、取り込む前に指摘や質問を返す工程です。GitHub や GitLab ではプルリクエスト(マージリクエスト)の画面で差分に行単位のコメントを付け、承認が集まってはじめて本流のブランチに取り込める、という運用が一般的です。
見る観点は主に4つあります。仕様どおり動くか、境界や異常系が抜けていないか、半年後の他人が読めるか、そして既存の設計や命名から浮いていないか、です。
なぜ必要か
自分が書いたコードの穴は、書いた本人が一番見つけにくいものです。頭の中に「こう動くはず」という前提があるため、その前提が間違っている箇所こそ読み飛ばします。他人が読むと、その前提を共有していないぶん素直に穴を踏みます。
もうひとつの効果は知識の分散です。ある機能を書ける人がひとりしかいない状態は、その人が休んだ瞬間に手が止まります。レビューを通すたびに、最低でももうひとりがその領域を読んだことになります。
具体例
指摘は、何が問題かと、なぜ問題かをセットで書くと通じます。
悪い指摘
「ここダメです」
「なんでこう書いたんですか」
通じる指摘
「items が空配列のとき items[0] で落ちます。
長さ0のケースを先に返す形にできますか」
「この関数名の get だと副作用が無いように読めますが、
中でDBに書いています。save 系の名前が近いと思います」# レビューで指摘されやすい書き方
def get_user_total(items):
total = 0
for i in range(len(items)):
total += items[i]["price"] * items[i]["qty"]
return total / len(items) # 空配列でZeroDivisionError
# 指摘を反映した形
def average_order_amount(items):
if not items:
return 0
total = sum(item["price"] * item["qty"] for item in items)
return total / len(items)つまずきやすいところ
差分が大きすぎるとレビューは形だけになります。1000行の変更に対して出る指摘は、100行の変更に対する指摘より少ない、というのが現場でよく起きることです。読み切れないので目が滑り、承認だけが押されます。目安として1回400行を超えたら分割を検討します。
指摘する側でありがちなのは、好みと問題を区別せずに並べることです。落ちる、遅い、危ない、といった直すべき指摘と、自分ならこう書く、という好みが同列に並ぶと、受け手はどれを直せばよいか判断できません。好みの指摘には「これは好みなので任せます」と添えます。
受ける側は、指摘をコードへの評価として受け取ると消耗します。読む相手はコードであって書き手ではありません。
似た用語との違い
| 語 | 対象 |
|---|---|
| コードレビュー | 変更差分を人が読んで指摘する |
| 静的解析 | ツールが機械的に規約違反やバグの型を検出する |
| ペアプログラミング | 書く時点で2人で見る。レビューを事後ではなく同時に行う |
