はじめに
以前に比べて人の書いたコードをレビューする機会が増えたので、普段レビューするときにどういったところを見ているか、何を考えているかを一度言語化して形に残しておこうと思います。
コードの中身について
「レビュー時に指摘できること ＝ 自分が書くときに気を付けられること」で、それ以上の指摘は出来ないと思います。レビューのやり方に悩んでいる人は、まず自分が良いコードを書けるようになりましょう。その上で本記事のように一度言語化をしてみると頭が整理できて良いと思います。
以下に自分が意識していることを列挙してみました。「リーダブルコード」と被っている箇所も多いので、そちらも参照ください。
-
誤字脱字 (typo)
- 放置する理由がない。
-
コードの見た目
- 一時変数への代入や横に長い行に改行を挟む、コードの「縦のライン」を揃えるなど。
-
命名
-
data,resultなどの抽象的な名前は避ける。
-
-
深いネスト
- 早期return・ガード節に直す。
-
適切な単位でメソッド、クラスに切り出す
- 長過ぎるメソッド、クラスはそれだけで読むのがコスト。切り出して適切な名前を付けましょう。
-
処理が冗長でないか
- 例： 求める結果に対してループが多すぎる、必要のない処理をループ内で行っている、不要な処理を早期returnで弾いていない、など。
-
不要なコードが残っている
- 削除差分に関連する使われなくなったコードは放っておくと埋もれてしまうので、不要なら今消しましょう。
-
コメント
- 単純な記述不足、記述とコードの乖離、言い回しが分かりにくいなど。
-
関連する機能への影響
- 修正対象に依存した別機能に影響がないか。
- ユーザー入力値のバリデーションがない
- テストコードがない
- 分かりやすい/親切なUI/UXであるか
心持ちの話
性悪説的にコードを読む
性善説的にコードを読んでしまうと、「XXさんが書いたコードならまずバグらないだろうしヨシ！」といった具合にザルめいたレビューになる恐れがあり、バグを生む可能性が高くなります。
コードレビューに臨む際は、「他人の書いたコードには大抵何かしらバグがあるし処理も改善の余地がある」という想定でいた方が良いです。仮にバグもなく書き方も完璧なコードだったとしても、そういった心持ちでいたほうがレビュワー自身もコードを真剣に読むようになると思います。
分からないことはすぐ聞く
レビュー依頼を受けたコードや周辺機能に自分が詳しいとは限りません。もちろんコードや（あれば）資料を読みはしますが、それでも実装の背景や意図が読み取りきれない場合もあります。そういった場合は長時間悩む前に開発者に質問しましょう（このとき「自分はどう理解したか」も添えると吉）。書いたコードに一番詳しいのは開発者自身のはずなので、欲しい答えが返ってくるはずです。
まとめ
- まずは自分自身が良いコードをかけるようになろう。
- リーダブルコードを読もう。
- 性悪説的にコードを読もう。
- 分からないことは開発者に聞こう。