概要
コードレビューにてコメントする際には、それが単なる自分の好みではないことや、どのように修正すべきかという意見を併せて伝えたい。
このため、私がコメントをするときには、できるだけ出典を明示するようにしています。
この記事は、過去に多く引用したページをまとめたものです。Javaに関するページが多めです。
フォーマットの説明
タイトルの頭についた【
数字】は重要度。
[5] Google's Engineering Practices Documentation
Googleのコードレビューガイドライン。
コードを書く側と、レビューする側の両方の視点に立って書かれている。
日本語翻訳もあり、読みやすいので、まずはすべて読み込むことをおすすめしている。
以下、特に引用する点を列挙する。この先のリンクは日本語翻訳版のみ。
コードレビューの基準
一般に、ある CL が作業対象のシステムのコード全体の健全性を具体的に向上させる状態に一度でも達したならば、たとえその CL が完璧なものでなくても、レビュアは承認を賛成しなければならない。
はじめてコードレビューする相手には、まず、大前提として伝える。
表現がまどろっこしいため、引用元を読むように伝えた上で、シンプルに「修正前より良くなっているならOK」と伝えている。
小さなCL
レビュアには、ただ CL が大きすぎるというだけの理由で無条件にリジェクトする決定権がある
「変更が大きすぎるので拒否したい」と伝えたい場合に引用する。
自分自身も、作成したリファクタリングの変更が、巨大なものになってしまったことがある。この原則に反していたため、小さな一連の変更(計13個)に分けて再提出したところ、レビュアーからは「読みやすい」という評判を得ることができた。
[4] 新人プログラマに知ってもらいたいメソッドを読みやすく維持するいくつかの原則
可読性を下げる書き方の例と、その解決策がシンプルにまとまっている。
この記事に書いてある原則を浸透させたり、自動チェックの機構を整備することで、指摘自体の数を減らせるようになるはず。
[4] 良いコードの書き方
「明らかなルール違反とまでは言えないものの、これは悪いコードだよな」と思ったときに、このページ内の本文を検索すると、大抵、問題点とその解決策が見つかる。
ちなみに、本記事の重要度の記法についても、この記事に倣っている。
[3] 効果的なレビューコメント
下記のような略語を初めて使う相手に対して。
- [Must] 必ず直すべき
- [IMO] 自分ならこうするけど、どう思う？(In My Opinion)
- [nits] 重箱の隅をつつくような、ほんの些細な指摘 (nitspick)
Mustのものが修正されており、nitsレベルを超えるような疑問点がなければ、「修正前より良くなっている」と判断して受け入れている。(IMOの場合は、それに対する返信をもとに判断する)
下記の記事でも同じように、略語を使って円滑にコミュニケーションするという内容が書かれている。
なお、チームでは、重要度について[Must][Should][Want]と表現することが多いため、これらに加えて[Should]や[Want]も使う場合がある。
[3] レガシープロダクトで始める仕様化テスト_コミット履歴の作り方の工夫
コミット履歴の作り方が悪かったり、小さなCLになっていないケースに対して、具体的な改善案を伝えたいときに。
[2] ふつうのJavaコーディング
「メソッド作成を面倒くさがらない」でほしいということを、サンプルコードをもとに伝えたいときに。
- 引数を増やす前にメソッド追加を考える
- 条件式よりメソッドを好む
- 同じオブジェクトを何度も呼ばない
- andやorはメソッドを抽出する
[2] Future Enterprise Coding Standards for Java
フューチャーアーキテクト社のJavaコーディング規約。
一般に利用・参照されているJavaコーディング規約やガイドラインに対するリンクも含まれる。
2020年4月現在、「Java コーディング規約」でのGoogle検索結果の最上位に表示される。
「こういう規約を定めているチームもあるみたいですよ」と、参考情報として引用する。
[2] Javadoc ドキュメンテーションコメントの書き方
ドキュメンテーションコメント(Javadoc)が足りない場合に。
特に、メソッドのパラメータや戻り値の仕様が明確でない場合に指摘をすることが多い。
[2] うまくクラス名を付けるための参考情報
うまくメソッド名を付けるための参考情報も併せて。
クラスやメソッドの命名に違和感があったときに、このページなどを引用しつつ、自分の意見をコメントする。
命名については、もうちょっとネタ元を充実させたい。
[1] I Shall Call It.. SomethingManager
もし、SometingManager Class1 が新しく作成されていたら、反射的に。。。
-
SessionManager, ConnectionManager, PolicyManager, QueueManager, UrlManager... ↩