ソースコードレビュー時に意識していること

もうすぐ春ですね、下條です。
弊社ではプルリクエスト(以降PRと書きます)ベースのソースコードレビューのプロセスがあります。今回はソースコードレビューをする際に私が気をつけていることをまとめてみます。

レビュー方法について

まずはレビュー方法についてです。

普段のレビュー

通常はGitHubなどのPRを皆が好きな時間にレビューしています。PRのコメントでレビューアにメンションを付けておくことになっているのですが、つい見逃してしまう場合があるので午後3時をレビュー時間としています。午後3時にSlackに通知が来るようになっています。
もちろん急ぎの時はお願いして急いでレビューしてもらいます。

対面 or 画面共有レビュー

対面レビューは実施コストは高いですが場合によっては有効と考えています。GitHubなどでのdiffを画面に写して一緒にレビューするというやり方が良いと思っています。私が昔勤務していた会社では紙に印刷して対面レビューをするようなことをしていましたが、、、プロセスとして重すぎると思います。紙に印刷するのはソース全体が見やすいという点でいい面もあるのですが。

対面レビューが適していると考えられるのは以下のような場合です。

• 重要ロジックを実装している場合。
• 複雑または斬新な実装で、レビューアが理解をするのに時間がかかるため、レビューイが直接説明したほうが早い場合。

流れとしては、レビューイが順に説明していき、レビューアが適宜チャチャを入れる形になります。指摘事項はその場で修正してしまえば後で再度レビューする手間も省けます。デメリットは人数×時間分の工数がガバっと必要になるところです。したがって、PR全部ではなく、重要箇所のみ対面レビューをするのがよいと思います。対面レビューでは以下のようなメリットがあります。

• 話しながら説明し、聞くことでロジックのバグに気付きやすくなる。
• 対面でない場合、すごく細かいまあどうでもいいやというような部分を指摘しない場合があることは否定できません。ただ対面だとそういったちょっとした部分もコメントしやすくなります。
• ワイワイガヤガヤやるのが楽しい。レビューにはもちろんバグを発見するという目的はありますが、レビューを通して議論をしながらお互いに成長するという目的も大きいです。そういう意味でやはりレビューは楽しくやりたいと思っています。

ただ、弊社の場合は皆がリモートで働いているため対面レビューをする機会は多くは作れません。その代わりにSkypeやGoogle Hangoutsで画面共有しながらレビューすることで代替とすることができます。

レビュー観点について

どこに重点を置くかは場合によりけりですが、私は以下の観点を中心に見ています。各自別々の観点を分担して見るという手もありますが、現状はそのようなルールは設けていません。

• 実装の目的
  弊社では、GitHubのPRにフォーマットを設けていて、実装目的を書くようにしています。まず、目的を再確認します。

• 機能要件を過不足なく実装・動作しているか
  まずは、きちんと機能要件が実装されているかを見ます。大きな機能追加などでは、実際に自分の開発環境で動かして確認することも多くあります。また、事前に設計した仕様には書いていないような細かい画面動作なども見ます。

• ロジックは正しいか
  上記項目とかぶる話です。複雑なロジックで、がんばって読むより話してもらったほうが分かりやすいと判断すれば、会話して確認することもあります。先の対面レビューのところでも書きましたが、実際に会話しながらレビューするとバグが見つかることもよくありますので。

• 実装が適切か(特にモジュール分割、クラス設計)
  例えばRailsの場合ですと、ロジックをモデルに入れるのか、もしくは別のサービスクラスなどに切り出すのかなど、クラス設計がイイ感じかを見ます。

• クラス名、メソッド名、変数名などは妥当か
  クラス名、メソッド名、変数名はソースコードの可読性に直結します。妥当な名前が付けられない場合は、より良い名前がないかどうか、もしくはクラスやメソッドの役割などがそもそも多すぎるのかなどといった部分を見ます。
  また、typoがないかどうかも確認します。typoは後になって効いてくることがあります。特にDBのカラム名をtypoしてたりすると、後の人が悲しむことになります。

• 適切な例外処理が入っているか
  例外時にどういう動きをするのかを想定しながら見ます。

• 性能
  データが多い場合に問題が発生しそうな箇所がないかを見ます。
  Railsの場合はN+1問題が発生することがあるため、Bulletであらかじめチェックしておきます。

• テストコードは書かれているか
  テストコードも一緒にレビューします。複雑なロジックがある場合にはテストコードもじっくり見ますが、それ以外の場合にはそこまでじっくりは見ず、ざっくりとテストの十分性についてのみ確認することもあります。

レビュー時に気をつけていること

以下のことを意識してレビューするようにしています。

• 指摘をするときには、 なぜ を書くことを意識しています。また、可能な限り修正案を提示するよう心がけています。

• あくまで対象はソースコードであり、人をレビューするわけではありません。あくまで客観的にソースコードを見るようにすることを心がけています。なお対面レビューでは、みんなで共通の画面を見てレビューすると、客観視しやすい気がするので、そういう意味でも大画面に写してレビューするのはオススメです。

• 指摘事項の書き方、言い方に気をつけています。コメント時には絵文字とかビックリマークとかを使うと、雰囲気が良くなると思っていて、特に最近は積極的に使うようにしています。

以上、レビュー時に気をつけていることを書いてみました。参考になる部分があれば幸いです。

なお、コードレビューにより品質を担保したサービス開発やモバイルアプリ開発を御希望の企業様は、是非MMMにご相談下さいませ！

shimo