コードレビューを行う際に自分が注意している点をまとめてみました。
余談ですが、最近リーダブルコードの本買いました。(どこかでブログに書くかも)
レビュ-の際の注意点
以下の観点に注意しながらレビューしています。
-
タイポ
-
共通化できるもしくは、共通化されているメソッドを使っているか
-
命名規則
-
言葉遣い
-
コメント
タイポ
当たり前といえば当たり前かもしれません。
また、メソッド名やエラーメッセージのタイポも丁寧に指摘します。
とはいっても、xxxが間違ってます!
といった命令口調ではなく
xxx -> yyy と思いますー
といったように修正後は
こんな感じになりますよと相手に優しく伝えるようにしています。
共通化できるもしくは、共通化されているメソッドを使っているか
これも、タイポと同じくらい注視しています。
例えば、チーム内で共通化されたメソッドがあればそれを使うように促したり、
共通化できそうなところは、参考コードを添えてレビューするようにしています。
命名規則
例えばブログサイトを構築していたとして、以下のような関数があったとします。
確かにブログを作成するための関数として create を選んでいると思うのですが、
これでは他の方が見たときに、何を作成するための関数なのか分かりずらいです。
func create(){
xxxx
}
長すぎず、短すぎず、汎用的すぎない命名になるので難しいところではあると思います。(自分もよく指摘される)
相手に分かりやすく伝えるような命名になっているか確認するようにしています。
なかなか決められない時は、codicなどのサービス使って作成するときもあります。
言葉遣い
以下の二つのコメントがあったとします。
<A>
ここはxxxのように修正したほうがいいです。
<B>
[IMO]
ここはxxxのように修正したほうがいいと思いました!
~~参考コード~~
どちらの方が相手に良い印象を与えられそうでしょうか?
A の方が命令口調でサバサバしていて、B はトゲがない感じがしますね。
また、B は参考コードまで載せているので相手の方はどのように
修正すればいいのか分かりやすくなっていると思います。
このように、ただただ間違いを指摘するのではなく
参考コードや相手を不快にさせないコメントの書き方をするだけで
良いレビューになるのではと思います。
また、B のコメントのように[IMO]
と添えてあげるだけで
どのような考えなのか可視化できると思います。
特に自分は、[IMO]
と[Q]
を使います。
- IMO...自分ならこのようにするかなーなど
- Q...実装で不明点などを聞く時(仕様がよくわからないなど)
コメント
意味のあるコメントと意味のないコメントを精査しながらチェックしています。
人によっては、コメントではなくコードを読めという人もいるかと思いますが、
私はコメントなら大歓迎なタイプです。
意味のないコメントでは以下のようなコメントが挙げられます。
// アカウントが存在する場合
if account {
xxx
}
パッと見たときに何をしているか判断できるものはコメントが不要かなと思います。
また、以下のような形で書く場合は修正を促しています。
// ブログ作成
func createBlog() {
}
あくまで Go の場合ですが、 // method名 コメント内容
で書くのが一般的かなと思います。
そのため、method 名があるかないかしっかり確認するようにしています。