この記事は何？
- レビューの際
- レビューをしてもらう際
上記の際に確認・気を付ける点を列挙し、理由を述べる。
内容はすべて自らの経験から。
対象者
業界3年目までくらい
セルフレビューをする
一度レビュアーにレビューしてもらう前にレビューイ自身で成果物を見直す。
不要なコミュニケーションが発生したり、単純なミスの指摘などを無くす効果がある。
レビューの基準が定まっている場合はそれに照らし合わせて自分の成果物を見てみる。
レビュー基準を統一する
レビューは非常に時間がかかるのでそれをいかに減らすか。
その場合レビューの基準をあらかじめ統一できていると判断する項目が減らせる。
不要な論議などをなくし、定めておこう。
複数人からのレビューをされる場合に不整合がおきることを減らす。
どうみるかを標準化しよう。
目的とするものは一つにする
複数の内容の変更は、ファイルがどれに対応するかなどレビュイーの負担が高い。
レビュー対象がなんの変更かがすぐわかるようになっているのであれば、レビュイーは考える事項がヘリ、レビューしやすい。
差分をなるべく対象の物だけにしてレビューを依頼するのが望ましい。
レビューされやすさ
- 何をレビューして欲しいのか
- なんの目的があるのか
- 特にどこの部分を見て欲しいか
- 問題はなんだったのか
- テスト結果はどうなったのか
フ- ァイルの関連はどのようになっているか
こう言った点がコメントに含まれているとレビュアーはレビュー対象が読みやすく判断しやすい。
テストを書く
誤字脱字・変数やメソッド名の間違えなどはテストを書けばなくすことができる。
実行できない、意図した動作が行われていない場合はコードレビューをする段階ではない。
仕様をコードにおこした形だけ確認するのであれば、コードを書く前に内容を確認してもらう方がよい、二度手間である。
ループを抜ける処理は上に書く
for文などでループを抜ける処理が発生するのであればできる限り、上の部分で書くほうが良い。
不要な変数へ値を格納する必要も処理を走らせる必要もない。
インデントを気にする
インデントが深いとコードを見る際に非常に不便である。
どこからどこまでが対象の処理なのか判別をする際に不便である。
例えば以下の場合はネストが深くならないように真偽判定の処理を見直すべきである。
foreach($result as $num => $info){
if(empty($test)){
//処理
}else{
//処理
}
}
foreach($result as $num => $info){
if(!empty($test)){
//処理
}
//処理
}
SQLを見直す
不要なデータを取得していないか、使う予定もないのに取得しようとしていないか。
使わないのであれば取得する必要はない。
責任分解できているか
メソッドに対して役割を付与しすぎていないか。
変数を判定する処理をそのメソッドでする必要があるのか。
必要以上の機能を付与していないか、それぞれの必要とする役割を考える。
以下が参考になる。
https://note.com/ywatanabe/n/n590d77662214
https://note.com/ywatanabe/n/n064f05bdd6c2
変数名は誰にでもわかるようにする
省略した形で変数を定義せず、そのままの形で表した方がわかりやすい。
コード補完もあるので長い変数名でも気にしない。
内容の意図をコメントに記載する
ここでのコメントはコードに挿入されるコメントではなく、
レビューをしてもらう際に連携するためのコメントである。
どんなに優秀なエンジニアであってもコードだけをみて仕様を把握することはできない。
どうしてもなぜこのようなことになっているのかわからない場合がある。
コードレビューをしてもらう際に処理フローについてのコメントを添えておけば、一手間も二手間も省くことができ、仕様についての質問なども最小限で済ますことができる。
特に、リモート環境となった世の中で、できるだけ応答回数を減らすことがそれぞれのパフォーマンスを上げることにつながる。
簡単でもいいのでどのような意図で処理を書いたか添えると相手が見やすい。
レビューはコミュニケーション
これが最も大事なことである。
相手に対して自分の意図を伝える、何がわからないかつ伝える。
具体的に適切に書く、だから何なのかを適切に伝えられるようになる。
負の感情を伝えない、そんなことをしてもなんの意味もない、注意を促せばそれで良い。
それらが守られるだけで有益なレビューになる。
参考
2021.1.1の更新はSDの良いレビューを行う技術を参考に行った。
https://www.amazon.co.jp/dp/B08Q82B3Y5