数百万年前、猿は木の上から地上に降り、その親指は他の4本の指と相対するように進化しました。そして最終的には人間へと進化を遂げたのです。
コードレビューの義務化を同様の観点から見てみましょう。つまり、ソフトウェア開発というサバンナに生きる動物であった私たちが、人間へと分岐するきっかけとなったのがコードレビューなのです。
とは言うものの、私はチームメンバの次のようなコメントを耳にすることがあります。
-
「このプロジェクトのコードレビューは時間のムダです」
-
「コードレビューをする時間がありません」
-
「私の担当箇所のリリースが遅れているのは、意地悪な同僚がコードレビューをしてくれていないせいです」
-
「私のコーディングを変更しろと言う同僚がいるのですが、どういうつもりなのでしょう？ この一点の曇りもない洗練されたコードに手を加えたりしたら、全体の繊細なバランスが崩れてしまうということを、あの人たちに説明してください」
なぜコードレビューをするのか？
最初に、なぜコードレビューをするのかを思い出しましょう。プロのソフトウェア開発者にとって最も重要な目標の1つは、ソフトウェアの品質を常に向上させ続けることです。たとえあなたのチームメンバが才能あふれるプログラマばかりだとしても、チームとして働く限り、有能なフリーランサーであることを切り離して考える必要があります。コードレビューはソフトウェアの品質を向上させる最も重要な方法の1つです。特に、以下の点において重要だと言えます。
-
不具合やより良い方法を見つけ出すための客観的な視点が得られる。
-
担当者以外に少なくとももう1人、コードの内容を理解する人を確保できる。
-
経験豊かな開発者のコードを見ることは、新人の訓練に役立つ。
-
コードのレビュアもレビュイも、他のメンバの良いアイデアやプラクティスに触れることで知識の共有ができる。
-
同僚のレビューを受けると分かっているので、開発者は自分の仕事に対してより完全なものを目指すようになる。
徹底的なレビューの実施
しかし、適切な時間配分とともに相応の注意を払ってレビューを行わなければ、上記の目標は達成できません。パッチに一通り目を通して、インデントが正しいか、全ての変数がローワーキャメル記法になっているかということを確認しただけでは、徹底的なコードレビューとは言えないのです。よく使われるプラクティスであるペアプログラミングの手法を検討してみることは有益でしょう。この手法では、基本的に開発と同じ時間をかけてコードレビューをするので、所要時間は全体の開発時間の100％増となります。このペアプログラミングに比べれば、どれだけ時間をかけてもエンジニア1人がレビューに費やす時間はずっと少ないのですから、コードレビューには十分な時間をかけることができるはずです。
私の考えでは、本来の開発に費やす時間のおよそ25％をコードレビューにかけるのが妥当だと思います。例えば開発に2日費やすとしたら、レビューには約4時間かけるべきだということです。
もちろん、レビューが正しく実施されていれば、かかった時間は重要ではありません。特に重要なのは、レビュー対象のコードを理解していなければならないということです。コードを記述している言語のシンタックスが分かっているだけでは十分ではありません。アプリケーションやコンポーネント、ライブラリなど、対象コードを含む全体のコンテキストとの適合性を理解している必要があります。コードの各ラインの密接な関連性を完全に把握していなければ、レビューはさほど意義のあるものにはならないでしょう。良いレビューをするのに時間がかかるのはそのためです。与えられた関数のトリガとなり得る様々なコードパスや、サードパーティのAPIが正しく使われているか（あらゆるエッジケースを含みます）などを調べるのは時間がかかるものです。
コードをレビューする際には、コードの不具合やその他の問題を探すだけではなく以下についても確認すべきです。
-
必要なテストケースが全てそろっている。
-
適切な設計書が存在している。
テストケースや設計書を書くのが得意な開発者であっても、コードを変更した時に、必ずしも他の関連ドキュメントを更新しているとは限りません。コードレビュアが適宜そっと更新への注意を促すことは、テストケースやドキュメントが古くならないようにするために必要不可欠なことなのです。
コードレビューの過重な負担を防ぐ
チームがコードレビューを義務化して行っている場合は、コードレビューのバックログが管理できないレベルにまで膨れあがってしまう危険性があります。2週間、全くレビューを行わなかったとしても、レビューの遅れを取り戻すための数日間を確保するのは、難しいことではありません。しかしこの状態では、いざコードレビューを行った時に、開発作業が大きな予期せぬ問題に巻き込まれてしまうことになるでしょう。また、きちんとしたコードレビューを行うためには、精神的に集中した状態を保たなければなければならないので、レビューの質を上げるのも大変です。何日も続けてこの状態を保つことは困難でしょう。
そのため、開発者は毎日、レビューのバックログを消化するように努力すべきです。朝一番にレビューを行うことは、それに対処するための1つの方法です。未消化のレビューをその日の開発を始める前に行うことで、レビューしなければならないと思い続けながら開発する状況から逃れることができます。昼休みの前後や、仕事の終わりにレビューを行いたいと思う人もいるでしょう。いつやるにせよ、レビューを開発の邪魔だと考えるのではなく自分の毎日の仕事として捉えることで、以下に挙げるようなことを回避できます。
-
レビューのバックログを消化する時間がない。
-
レビューが終わっていないためにリリースが遅れる。
-
コードの内容が既に変わっているのに、古いコードについて指摘してしまう。
-
大急ぎでレビューをしたために、不十分なレビューになってしまう。
レビューしやすいコードを書く
管理できなくなってしまったバックログについては、レビュアに常に責任があるわけではありません。私の同僚が大きなプロジェクトの開発をしていて、1週間、行き当たりばったりでコードを書いているとしたら、そのパッチは非常にレビューしづらくなるでしょう。1セッションで処理することが多すぎるでしょうし、コードの目的や根本的なアーキテクチャを理解するのが難しくなることが予想されます。
今述べたようなことは、管理可能な単位まで仕事を分けることが重要だと考える理由の1つです。私たちは自分たちの適切な仕事の単位がストーリーになるように、スクラム手法を採用しています。ストーリーによって仕事を整理する努力をし、今まさに作業している特定のストーリーについてのレビューだけを計画することで、格段にレビューしやすいコードを書くことができるのです。もちろん他の手法を使うこともあるかもしれませんが、根底にある考え方は同じです。
他にもレビューしやすいコードを書くための前提条件があります。もしアーキテクチャに複雑なところがあるならば、事前にレビュアに会ってそれについて議論しておくことは道理にかなっているでしょう。これによりレビュアは、コードが何のために書かれていて、機能がどのように実現されるのかが分かるので、コードをより簡単に理解することができます。また、コードを書く側にとっても、レビュアから別のより良いアプローチを提案されたことにより、コードの大部分を書き直さなければならないというような状況を回避することができます。
プロジェクトのアーキテクチャについては、詳細に設計書で説明すべきです。いずれにせよ、新たに開発に加わった人が開発のペースに追いつき、設計書を見れば既存のコードベースについて分かるようになっているのは重要なことです。レビュアにとっては、設計書を参照すれば正確な仕事ができるという、更なる利点もあります。また、単体テストは、コンポーネントがどのように使われているのかをレビュアに示す手助けになるでしょう。
パッチにサードパーティのコードが含まれていたら、別々にコミットしてください。9,000行のjQueryが真ん中にドロップされた場合にコードを適切にレビューするのは非常に大変です。
レビューしやすいコードを作成するために最も重要なステップの1つは、自分のコードレビューに注釈をつけることです。つまり、自分自身でレビューして、何が起こっているかをレビュアが理解するのに役立つと思われる箇所にコメントを加えるのです。私は、コードに注釈をつけるのは、比較的時間がかからないのに（大抵はわずか数分で）、どれくらい迅速にうまくコードをレビューできるかに非常に大きな違いをもたらすということに気づきました。もちろん、コード中のコメントには同じような利点がたくさんあるので必要に応じて書いておくべきですが、レビューして注釈をつけると、もっと役に立ちます。おまけに、開発者は自分のコードを読み返したり注釈をつけたりする間に、そのコード内にある多くの不具合を見つけるということが研究で明らかになっています。
大規模なコードのリファクタリング
多くのコンポーネントに影響を及ぼす方法でコードベースのリファクタリングが必要なこともあります。大規模なアプリケーションの場合、これに数日（またはそれ以上）かかってパッチが巨大になることがあります。このような場合、標準的なコードレビューは実用的ではないかもしれません。
最善の解決策はコードのリファクタリングを徐々に行うことです。妥当な範囲の変更を見つけ出してください。結果的にコードベースがきちんと機能し、目指す方向に向かうような部分的な変更です。その変更が完了してレビューが終わったら、次の変更に取りかかり、完全に終了するまで徐々にリファクタリングを行います。これはいつも可能だとは限りませんが、考えて計画すれば、通常はリファクタリングの際にパッチが巨大になってしまうことを避けるのは現実的なことです。開発者がこのようにリファクタリングを行うのはもっと時間がかかるかもしれませんが、レビューを容易にするだけでなく、より質の高いコードにつながります。
コードのリファクタリングを徐々に行うのが全く不可能ならば（それは恐らく元のコードがどれくらいうまく書かれてまとまっているかによりますが）、リファクタリングに取り組んでいる間にコードレビューではなくペアプログラミングを行うことが1つの解決策になるかもしれません。
論争の解決
あなたのチームは間違いなく知的なプロで構成されていて、ほとんどの場合、特定のコーディング問題について意見が異なっても合意できるはずです。レビュアが別のアプローチを好む場合に備えて、開発者として広い心を持って妥協する準備をしておきましょう。自分のコードに対して自分のものであるかのような態度を取らないでください。また、レビューのコメントを個人攻撃と受け取らないでください。あなたに対して、いくつか重複したコードを他の関数を再利用してリファクタリングを行うべきだと感じる人がいるからといって、あなたが魅力的でもすばらしい人物でもないと否定されているわけではありません。
レビュアとして気を利かせてください。変更を提案する前に、それが本当に優れた提案なのか、それとも単なる好みの問題なのかをよく考えましょう。議論すべき箇所を選んで、元のコードで明らかに改善が必要な箇所に論点を絞れば、もっとうまくいくでしょう。「ペットのハムスターの方がこれより効率的なソートアルゴリズムを書くことができる」と言うのではなく、「～を検討する価値があるかもしれません」や「～を薦める人がいます」という風に言ってください。
妥協点を見つけることができない場合は、両者が尊敬する他の開発者に見てもらって意見を聞くといいでしょう。