コードレビューのやり方、基礎の基礎 - コード改善に重要なレビューの基本的な考え方を学ぼう

コードレビューはソフトウェア開発活動の1つで、いくつか目的があります。もっとも一般的な目的は、コードを目で追って問題¹がないか確かめることです。自分の書いたコードをほかの開発者が確認したり、ほかの開発者が書いたコードを自分で確認したりします。

そのほかの目的として、他のメンバーのコードを理解して、自分のコードとの間で齟齬がないかを確認したり、問題がある部分の代替案を考えたりする場合もあります。1回のレビューでこれらすべてを目的にすることはあまりなく、1回目のレビューでは欠陥がないかを確かめ、2回目のレビューでは代替案がないかを確かめる、といった具合に使い分けるのが一般的です。

コードの「読み進め方」

どのような目的のレビューでも、確かめる箇所と確かめ方を決めてからレビューすると効率的ですし、レビューの質も上がります。このような、コードの「読み進め方」が定まっていないと、ただ漠然とコードを読み流してしまいがちです。

ただし、こうした読み進め方を手当たり次第に身につけて、とにかくたくさんの問題を指摘できるようになるだけでは「レビューのスキル向上」とは言えません。レビューのスキルは、大きく2つに分けることができます。

1つは問題を見つけるスキルです。読み進め方をたくさん知ることも含まれますが、それを適切に選ぶことやチームで合意することも含みます。もう1つは見つけた問題をうまく指摘するスキルです。見つけた問題を、どのように伝えるかで、問題が解決するスピードや精度は変わります。迅速に問題を解決するには、コードを書いた人をはじめ、チームに問題をうまく伝える力も求められます。

レビューは見よう見まねで続けられていることが多い

個々のコードレビューで、「読み進め方」が具体的に決められていることはあまり多くありません。過去にそうやっていた、ほかの開発者がやっているからといった理由で、なんとなく決まっている、という状況もあるでしょう。

レビューで指摘できることはかなり広範囲にわたるため、本来見つけ出したい問題以外の問題が指摘されることもあります。たとえば、再設計して実装をやり直すことが決まっているのにコードの拡張をしやすくするための指摘をしていたり、将来においても性能が求められないにもかかわらず、性能向上すべき点を指摘したりしていることもあります。

これからやろうとしているコードレビューの目的を理解し、「読み進め方」を選ぶことでレビューの質を高め、効率化にもつなげることができます。

適切な方法でレビューを進める

「レビュー指摘」という名目で、コードを書いた開発者を責めてしまうことは少なくありません。「必要ないかもしれないけれど、自分のコードレビューのときには言われたから」といった理由で、理不尽な指摘や要求をしているのを見たことはないでしょうか。また、自分のほうが技術的な知識が多いことを示すことが主な目的となって、必要のない指摘をしているというケースに遭遇した経験を持つ方もいるでしょう。

こういった、プロダクトの質を高めることにつながらない指摘を避けることは、レビューで求められる基本的なスキルです。なぜなら、レビューの効率化につながるからです。さきほど挙げた例のように、プロダクトの質とあまり関係のない問題を見つけようとしていれば、まずは目的を見直す必要があります。

レビューの目的を理解する

ただ漠然とコードをながめても、ある程度はレビューを進めることができます。その一方で、コードの分量が多くなったり、時間制約があったりすると、レビューが難しくなります。 そのような場合には、以下の例のように、問題が起きそうな部分を中心に確認します。

例1)別々の開発者が書いたメソッドや、関数間でやりとりされるデータに不整合はないか 例2)入力値やパラメータによっては実行時間が極端に長くならないか

おおまかに「ここでこの問題があったら、こういうバグにつながる」という問題がイメージでき、その問題を見つけるための「読み進め方」がイメージできていれば、レビューの目的が決まったと言えるでしょう。

目的を決めることで、"近々再実装することが決まっているのに、拡張性に関する問題をたくさん指摘される"とか、"性能やセキュリティ上の問題がないかを確かめることが重要なシステムにおいて、拡張性の問題をたくさん指摘される"といったレビューの行き違いが防げます。

複数人でレビューをするときは、ほかのレビューアと同じ目的を共有しているか確かめてください。目的が決まっていなければ、そのレビューに求められる目的を話し合い、事前に合意しましょう。

組織標準のチェックリストがある場合や目的が決まっている場合は、それに沿って進めます。レビュー会議で「そのタイプの問題は、今回は指摘する必要はないと思う」といった議論が始まると、レビューの時間がなくなってしまいます。また、参加者が「そういう問題は指摘しなくてもいいのでは」と感じてしまうレビューも同様です。こうしたことが起こらないように、レビューを始める前に目的を確認・合意しておくのが重要です。

問題を見つけよう

コードレビューの原則は、あるべき姿とコードを対応づけて問題を見つけることです。自分なりの「読み進め方」を身につけると、問題がないかどうかをすばやく判断できたり必要のない箇所を読み飛ばしたりできます。そうすると、問題を早く見つけることができます。

では、「読み進め方」の具体例を見ていきましょう。本記事では、3つのタイプを紹介します。

読み進め方その1 「バグがなさそうか」を確かめる

1つ目のタイプは、バグ（期待する動作をしない）があるコードです。もっともわかりやすく、あるべき姿とコードを対応づけてバグを見つけるための「読み進め方」です。オンラインショッピングサイトの開発であれば、以下のようなバグが考えられます。

• 画面で指定した商品とは違う商品が注文される
• 画面で指定した数量とは違う数量の商品が注文される
• 登録されている商品名とは違う商品名が表示される

これらを実現するコードがどこにあるかを特定して、正しく実現できているかどうか確かめましょう。テストコードがあれば、そのテストコードでバグが見つかるかどうかも確認します。 このような問題を見つけるための確かめ方は、以下の3つをチェックすることです。

• 指定した商品と同じかどうか
• 数量が同じかどうか
• 登録されている商品名と表示される商品名が同じかどうか

そのため、商品がどのように識別されるか（IDでの管理やそれが重複しないためのしくみ、検索時にどのデータ項目が検索対象になるかといった点）、数量はどこで指定され、処理されるかといった点を確認します。 バグの検出はテストでも行えますが、コード上でないと見つけにくいバグや大規模な修正が必要になりそうなバグは、コードレビューで確認するとよいでしょう。

レビューでは、バグのような「期待する動作との違い」だけではなく、ある条件下で問題になるようなコードや、将来の拡張において問題になりそうなコードといったタイプがないかを確認することがあります。それぞれ見ていきましょう。

読み進め方その2 「ある条件下で問題になる箇所」を確かめる

2つ目のタイプは、ある条件下で問題になるようなコードです。以下の図はC、C++、Javaをはじめとするさまざまなプログラミング言語で書ける多重ループの例です。数値v1、v2を与えて、clearという関数を呼びます。clear関数の実行内容やv1、v2の値によっては、実行時間が長くなる可能性のあるコードです。v1やv2が10以下であれば、clear関数の実行は100回を超えることはありません。しかし、v1とv2が両方とも1000を超えると、clear関数の実行は100万回を超えます。そのため、clear関数の実行時間と勘案して、100万回を超えて実行される場合でも問題がないか、もし問題がある場合にはこの書き方でないと実現できないかどうかを確認し、よりよい書き方を考える必要があります。

void test (int v1, int v2) {
    int i, j;
    for (i = 0; i < v1; i++) {
        for (j = 0; j < v2; j++) {
            clear(i, j);
        }
    }
}

ループの回数によって時間がかかる可能性があるコードを見つける読み進め方は、以下のようにまとめられます。

• 確かめる箇所：多重ループ
• 確かめ方：ループの繰り返し回数によっては極端に実行時間が長くならないかどうかチェックする

読み進め方その3 「コードの拡張性」を確かめる

3つ目のタイプは、将来コードを拡張する際に、コードを読む人を勘違いさせやすいコードです。コードを読む人にとって、紛らわしい書き方をしていないかどうかを確認するための読み進め方を紹介します。

次のコードを見てください。C、C++、Javaで書ける、紛らわしい条件分岐の書き方です。コードの3行目には2つのif文が書かれています。1つ目のif文の条件が満たされているときには、value++が実行されます。その次のif文は1つ目のif文の条件が満たされていないときでも実行されます。しかし、1つ目のif文の条件が満たされているときだけ実行されるようにも読めます。つまり、2個目のvalue++がv1とv2の両方がNULLのときにしか実行されないのか、v2がNULLであれば（v1がNULLでなくても）実行されるのかがわかりにくくなっています。

int test (int v1,int v2) {
    int value = 0;
    if (v1 == NULL) value++; if (v2 == NULL) value++;
    return value;
}

紛らわしい条件分岐を見つける読み進め方は、以下のようにまとめられます。

• 確かめる箇所：if文の後の中カッコを省略しているところ
• 確かめ方：条件を満たしたときに実行される文が、紛らわしく記載されていないかどうかチェックする

では、このコードをどのように修正すれば、勘違いが起こりにくいコードになるでしょうか。2つの改善案を紹介します。コードレビューのやり方によっては、こうした改善案を提案する場合もあります。

1つ目のif文の後のvalue++の後で改行（改善案1）するか、value++の前後に中カッコをいれて明示（改善案2）したほうがわかりやすくなります。

改善案1

int test (int v1, int v2) {
    int value = 0;
    if (v1 == NULL) value++;
    if (v2 == NULL) value++;
    return value;
}

中カッコを追加したコードが次のコードになります。

改善案2

int test (int v1, int v2) {
    int value = 0;
    if (v1 == NULL) {value++;} if (v2 == NULL) {value++;}
    return value;
}

地道ではありますが、こういった「読み進め方」を増やしていくことで、「問題がないかをこのくらい確かめておけば大丈夫」という実感が持てるようになります。そうした実感がなければ、「1時間見たからいいか」というような所要時間ありきのレビューになってしまいます。漠然とながめていていも、確認すべきところをじっくり見ても、時間はかかります。レビューの質を安定させるために、「読み進め方」を増やしていくのが重要なのです。

別のところにも流用できる読み進め方が増やせていても、最初は、どれも違う問題に見てしまい、同じような問題が見つからないのではないかと思いがちです。しかし、読み進め方のバリエーションを増やしていくと、あてはまる問題が出てきます。とくに似たようなプロダクトやドメインの開発をしていれば、読み進め方を流用できる機会が増えます。もちろん、レビューだけではなく、自分でコードを書く場合にもあてはまることが増えます。

見つけた問題を指摘しよう

コードレビューで見つけた問題は指摘して、コードの改善を促します。指摘のしかたはレビューの形態により変わりますが、形式の大分類として、以下の2つがあります。

• ツール形式（ツールを介したレビュー）
• 会議形式

それぞれのやり方と注意すべき点を見ていきましょう。なお、どちらの形式であっても、問題の見つけ方に違いはありません。

ツール形式のレビュー

ツール形式では、見つけた問題を説明する文章をテキストとして入力し、コードリポジトリの管理ツールや、メールといったツールを介して、コードを書いた人に伝えます。 ツール形式のレビューのメリットは、レビュー参加者の時間を合わせる必要がない点が大きく、そのため、短期間でレビューを終わらせやすくなります。また、記録が残しやすいので、修正において指摘に対応しやすい点もメリットです。

デメリットは、誰も問題を確認していない場所があっても気づきにくかったり、複数のレビューアが同じ問題を指摘したときにムダが多かったりする点です。また、会議形式で行えば、口頭で簡単に説明できる内容であっても、ツール形式のレビューでは説明するためのテキストを考えるために時間がかかったり、レビューアの意図がうまく伝わらなかったりすることもあります。

GitHubなどのツールを活用したレビューでは、修正案（修正後のコード、GitHubならばPull Request）を送ることが一般的です。

こうしたツール形式のレビューで気をつける点は、ほかのレビューアの指摘を見る機会が減ってしまう可能性があることです。あらたに「読み進め方」を知ったり、よりよいレビューのために、ほかのレビューアの指摘から学んだりすることも多いので、積極的に見るようにしましょう。

レビューツールには、いくつかの種類があります。GitHubは、ソースコードバージョン管理システム（ソースコードリポジトリ）にレビュー機能が付属している代表的なツールとして、多くの方が知るところでしょう。図1は、GitHubでのレビューアと開発者（作者）のやりとりの様子を表示させたものです。特定環境でのインストールスクリプトに問題があり、改善案（パッチ: patch）とともにレビューアが指摘してくれたコードを作者が取り込んでいる様子です。画面中央部にある「38a258a」のリンクをたどると図2のようなパッチと現行バージョンとの差分を表示してくれます。

図1 GitHubでのレビュー指摘の例（https://github.com/motemen/ghq/pull/82）より

図2 パッチの表示例（https://github.com/motemen/ghq/pull/82/commits/38a258a2c229f7bb7efd30825639f66a5a2d24bc）より

ソースコードリポジトリを使った開発を前提としない、レビュー専用ツールもあります。ソースコードリポジトリを使った開発では、開発プロセスの一部をツールが想定するプロセスに合わせる必要がありますが、レビュー専用のツールでは、その必要はありません。図3はそうした専用ツール製品の1つであるLightning Reviewの画面です。

図3 レビュー専用ツールでの指摘と修正の例（Lightning Review）

ツール形式のレビューには、課題管理ツールやバグ管理ツール、エクセルのような表計算アプリケーション、メールも使えます。

レビューは、コードを書いたメンバーとテキストメッセージをやりとりすることでもあります。テキストメッセージのやりとりにおける注意点は後述します。

会議形式のレビュー

会議形式では、見つけた問題を会議の場で発言して指摘します。レビューを短時間で終わらせるために、問題の指摘を中心にし、問題の修正は後回しにするかコードを書いた開発者に任せることが多いです。 現場やプロジェクトによってはツール形式のレビューが多く、会議形式のレビューは少ない場合がありますが、会議形式のメリットは指摘の内容が理解できないときには、その場で聞き直せたり出席者で考えたりすることで、その場で解決したり共通理解を得られたりしやすい点です。必要であれば、修正方法に関しても関係者で話し合うことができます。

問題を文章（テキスト）としてツール経由で説明すると手間がかかる場合でも、会議形式でインタラクティブに説明できれば、簡単に伝わるメリットがあります。ツールを使ったレビューと会議形式のレビューを比較した研究論文²では、会議形式のほうが指摘誤り（正しい内容を誤って問題として指摘すること）が減ることが報告されています。

デメリットは、レビューの実施日時の調整やレビューを実施しようとしてから、実際に会議を開始するまでに時間がかかる点です。また、会議の時間配分をうまく調整しないと、コードの説明だけで終わってしまったり一部しかレビューできなかったりするデメリットもあります。

会議形式のレビューで気をつける点は、慣れないうちは事前に準備しておかないと、レビュー中に問題を指摘できないことです。会議で初めてコードを見てすぐに問題を見つけるには、トレーニングが必要です。事前にレビュー対象のコードを確認できるように時間を確保しておくといいでしょう。

気をつける点は、もう1つあります。せっかくほかのメンバーの指摘を聞く機会であるにもかかわらず、自分のコードが対象でないときは聞き流してしまうことです。ツール形式のレビューと同様で、ほかのメンバーの指摘を聞き流すと「読み進め方」のバリエーションを増やしにくくなります。「自分だったらどうするか」「自分の書いたコードにあてはまることは？」といった視点でほかのメンバーの指摘に耳を傾けると、レビュー会議がより意義深くなるでしょう。

レビュー指摘はテキストメッセージと捉えよう

ツール形式、会議形式いずれの場合も、発見した問題はなにかしらの形で指摘が行われます。あなたが指摘する側の場合、注意すべきは指摘の伝え方です。ここで大事にしていただきたいのは、レビュー指摘はテキストメッセージと同じである、という点です。そもそもテキスト主体で運用されるツール形式のレビューでは、とくに意識すべき点です。会議形式のレビューでは、問題を伝えたい相手の様子がすぐにわかるので、適切でない伝え方をしてしまった場合、その場で変更したり修正したりできるチャンスがあります。しかし、テキストの行き来で成立するツールを使ったレビューの場合には、そうしたチャンスは失われがちです。

テキストメッセージでの失敗を経験し「この書き方では真意が伝わらない」、「この書き方では問題がある部分を書いた開発者を傷つけるかもしれない」といった配慮をするようになった方は多いでしょう。とくにツール形式のレビューの場合には、「口頭で伝えるとして、この言い方をするか」「口頭では伝わる誇張や冗談がテキストにしたときも同じように伝わるか」といった確認をしてから、指摘を送信（提出）するようにします。

図1は適切なやりとりの例です。最初に問題点が具体的なエラーメッセージとともに指摘されています。ここで実行環境を示さずに「make³が失敗する」とだけ書くとなかなか伝わりません。オープンソースソフトウェアのレビューやバグレポートでは一般的なやり方ですが、修正案とともに示すことで、建設的なレビューにできるのです。また、作者が指摘に感謝の一言を加えて改善案を取り込んでいます。こうした配慮がツール形式のレビューをより円滑に進めます。

気をつけたいのは、「こんなこともできていないのか」と思うような指摘をするときです。問題の解決に必要のない表現や言い回しになっていないかを確認します。「この程度のことも書けていない」や「性能向上のつもりかもしれないが」といった言葉は、書かなくても問題の内容は伝わるはずです。こうした記述が指摘の中に紛れ込んでいないか、投稿前に必ずチェックしてください。

自分が開発した成果物に対して、こうした表現で指摘があった場合には、その指摘をあえてメモしておきましょう。自分が指摘する際にも同じような表現をしていないか、チェックリストとして使います。そうした「レビュー指摘の不適切な表現」を、アンチパターンとして学ぶことで、将来、自分が指摘する際にそういった指摘を含めないで済むようになると考えてください。そうすると、本来の目的である問題の修正（指摘された内容の反映）に集中できるようになるでしょう。

コードレビューをしてもらうときの心構え

ここまで、主にコードレビュー自体やレビューする側の考え方について紹介してきました。最後に、コードレビューをしてもらうときの心構えを紹介しましょう。

自分が書いたコードをレビューしてもらい、コードの問題を指摘されるとどうしても自分の技術やスキルが批判されているように感じてしまいます。こうした感情を避けるために、自分が間違えてしまった理由や、ほかに考えたことを説明して、ミスした理由をわかってもらおうと思うときもあるでしょう。しかし、本来やるべきことは、指摘をすばやく理解して、適切なコードに修正することです。問題を指摘した開発者（レビューア）に、ミスをした理由をわかってもらおうとして説明するだけでは、適切なコードになることはありません。もちろんその開発者が誤解している場合は例外です。正しい理解をしてもらうために説明をします。

ミスした理由をわかってもらおうとするのは、それを自分の実力だと思ってもらいたくなかったり、自分の評価につながってしまうと考えてしまったりするからではないでしょうか。レビューアとの会話や接点が少ないと、どうしても自分の書いたコードで自分自身が評価されてしまうように感じてしまいます。よりよいコードを書くことは言うまでもありませんが、カジュアルな会話やコードレビュー以外の接点を持つことで、「いろいろ考えて作業を進めているんだな」とレビューアに思ってもらうことで、レビューの場だけで自分が評価されるわけではないと前向きに考えることができます。

厳しすぎる指摘を見ると「だったら自分で書けばいいのに」という気持ちになることもあるでしょう。そのときは、「批判だけならば誰でもできる」と考えることをお勧めします。もちろん、レビューアにその考えを伝える必要はありません。実際のところ、一通り動くコードを作ることは、細かい指摘をするよりもとても大変なことです。レビューアはそのことを十分に理解した上で指摘していると考えればコードの修正に集中できるようになります。

まとめ

さて、コードレビューの基礎をお伝えしました。レビューの工夫や読み進め方は拙著「間違いだらけの設計レビュー」や私のTwitterアカウントでも紹介していますので、さらなる情報が必要な方は、ぜひご覧ください。

多くの先達も、同じようにレビューを苦手に感じてしまったり、レビューを評価の場と捉えてしまい、へこんだりした経験があるものです。そうしたことがあまり気にならなくなったり、他のレビューアの指摘を見て自分の再発防止につなげたり、「そういう読み進め方があるのか」と思うようになれれば、レビューはつらい作業ではなくなり、積極的に参加しようという考えに変わってくると思います。この記事がきっかけで、皆さんの明日からのレビューが実り多いものになれば幸いです。

森崎修司 （もりさき・しゅうじ） smorisaki

名古屋大学 大学院 情報学研究科 准教授。ソフトウェアレビュー、ソフトウェア計測を専門とする。コードレビュー（ソフトウェアレビュー）、ソフトウェア計測、実証的ソフトウェア工学に関する執筆活動多数。主な著作に『なぜ重大な問題を見逃すのか？間違いだらけの設計レビュー』（刊：日経BP）がある。

編集：横田恵（リブロワークス）