コードレビューの品質を上げよう

スポンサーリンク

コードレビューをもっと良くしようという経験的な記事です。

コードレビューの目的

コードレビューの主たる目的は、コードベース全体が常に改善し続けていることを確認すること にあります。どんなツールもプロセスもこの目的のためにあります。

改善しているかどうかは次のような指標で測る:

  • maintainability
  • readability
  • understandability

原則としてレビュアーは提案された変更がコードベースを改善することがわかったらapproveします。完璧を求める必要はなく、継続的な改善になっているかが鍵です。

コードレビューの原則

  • 意見や個人の嗜好よりも技術的な事実やデータ
  • スタイルに関してはスタイルガイドのみが権威を持つ。スタイルガイドになければ個人の好み。スタイルはそこで一貫しているべき。もし前例がないのであればauthorを受け入れる
  • 適切なコメントを残す。言語やフレームワーク、ソフトウェアデザインの原則を学ぶ重要な機会でもあるので、コメントを残し、知識を共有することでコードヘルスは良くなる
  • 揉めたら実際に会うかビデオで話す。また、会話の内容は記録しておく。解決しない場合はチームでの議論にする

コードレビューピラミッド

The Code Review Pyramid
When it comes to code reviews, it’s a common phenomenon that there is much focus and long-winded discussions around mundane aspects like code formatting and sty...

コードレビューでは、フォーマットやスタイルの議論に焦点が当てられ、想定通りの実装になっているか、パフォーマンスが良いか、既存実装に影響がないかなどの重要な議論が蔑ろにされがちであると言ってます。

この問題を解決するために、コードレビューで大事にされるべき観点をピラミッド形式で表現した図が公開されています。

コードレビューピラミッドは、あとで変更するコストが大きいものから小さいものまでレイヤ分けされています。各レイヤで投げかけるべき質問の例もあります。さらに、どの範囲までを自動化して、どの範囲にコードレビューの時間を使うべきかも示されています。

ざっくりとしたレビューの観点

ロジックをレビューする

主に、仕様を満たした実装であるかを見ます。境界条件を確認したり、実際に動作確認を行うなどのレビューをします。

リーダビリティをレビューする

可読性(リーダビリティ)とは、つまり読みやすさのことです。一般的な可読性から開発言語に特有の書き方などのレビューです。

設計をレビューする

設計レビューと呼ぶのが適切かは分かりませんが、コードの責務が適切か、既存の実装に不都合な実装になっていないかなどをレビューします。将来的な拡張性もレビューできると良いでしょう。

具体的なレビューの観点

デザイン/設計

コードレビューで最も重要なのは全体のデザインです。

それぞれの変更は意味がありますか?コードベースに対する変更ですか、それともライブラリの変更ですか?システムの他の部分とうまく連動しますか?その機能は今追加するべきですか?

機能

その変更は想定通りの機能を果たしていますか?
開発者がユーザにとって良いと思っていることはなんですか?ユーザはエンドユーザであることもあれば、今後コードを使うであろう未来の開発者であることも。

開発者が十分にテストしていることが望まれますが、レビュアーとしてもエッジケースに注意し、並列処理の問題がないか調べて、ユーザの立場で考え、コードにバグがないかを調べる必要があります。

UIに変更がある場合などは、実際に変更が合っているかを確かめることもできます。どうやって確かめれば良いか分からない、あるいは簡単には再現できない場合には開発者にデモを用意してもらって確かめましょう

もうひとつ重要なのは並列プログラミングの問題です。デッドロックやレースコンディションがある場合には念入りにチェックします。しかしこうし問題は発見が難しいのでできれば避けたほうが良いでしょう

コードの可読性

仕様を把握していて実装するときには気付きづらいですが、「あとから読んで理解できるか」、「仕様を知らない第三者が読んで理解できるか」、そして「すぐに理解できるか」を気にします。

命名

変数や関数名が適切かです。

あまりに抽象的な命名だと、どういう目的の変数や関数なのかを見失ってしまいます。悪い場合には他の変数と重複してしまうことも。

命名する際には情報量を多く、できるだけ省略しないようにするのが良いでしょう。

関心の分離

再利用性を高めたり、バグを減らすことができます。

一つの関数が責務を負いすぎていないか、不適切な責務まで担っていないか、採用しているフレームワークなどと相談して適切に分割されるようにします。

非機能要件

一度処理してしまうと取り返しがつかないもの、メールの送信や決済などは念入りに。動作確認やテストは十分だろうか?
もし設計変更があったときに変更しやすいだろうか?

以下は未整理の観点。

  • 複雑さ
  • テスト
  • コメント
  • スタイル
  • 一貫性
  • ドキュメント
  • everylin
  • context文脈
  • good things
  • コーディング規約に準拠しているか

レビューコメントの仕方

まずは基本的に攻撃的にならないように心がけましょう。コードを書いた人にはこだわりがあるかもしれないことは念頭に置いた上で、議論をします。

コメントの温度感

また指摘する内容の重さも伝えると良いと思います。必ず変更するべきなのか、提案なのか、それとも疑問なのか。自分は質問する形式にすれば角が立ちにくいと思っています。

  • MUST – 必ず直してほしいところ。どう直して欲しいのかも具体的に書く。理由も。メンテしづらくなったりパフォーマンスが悪かったり。
  • IMO – In my opinion。「自分ならこう直す」というような提案型のコメント。メリデメが両方あるような場合のものに、具体的なコメントをつけて書くと良い。
  • NITS – 直しても直さなくてもいい程度の修正につける。
  • ASK – 実装ないようがわからないとき。
  • GOOD – 実装が良いときに書く。気分が良いので。

指摘だけでなく改善案も

また改善案はできる限り付け加えます。

参考リンクをつけよう

参考になるものがあれば、リンクもつけます。言語仕様だったりコーディング規約があると良いでしょう。

コードレビューで使える略語集

  • LGTM – Looks good to me.approveするときに使えます
  • FYI – For your information. 参考情報を提示できます
  • NIT – nitpick. 細かいことを言うようで恐縮ですが。not important though とも。

参考文献

How to do a code review
Google’s Engineering Practices documentation
eng-practices/index.md at master · google/eng-practices
Google's Engineering Practices documentation. Contribute to google/eng-practices development by creating an account on GitHub.

Googleのコードレビューガイドラインです。

コードレビューの際に気をつけること - Qiita
コードレビューをする際と、してもらう際に気をつけるべきだと思っているをまとめておきます。 レビュイーとして大切なこと コードレビューをお願いする前に レビュアーが高いパフォーマンスを発揮するためには、レビューを受ける人の心構...
コードの不吉な臭い - Qiita
新装版 リファクタリング 既存のコードを安全に改善するを改めて読んでみて「第3章 コードの不吉な臭い」が面白かったので、読書ログとして残しておく。 コードの不吉な臭いとは コードに深刻な問題、リファクタリングを必要とするコー...

こちらはコードレビューの指摘対象。

よいポストだった。

タイトルとURLをコピーしました