多くのWeb系企業では、新人、ベテラン関係なく他人のレビューをする機会があります。
しかし、コードレビューのやり方は教わることがなく「何を見たら良いのかわからない」と悩む方もいるのではないでしょうか。
僕も新卒でエンジニアになったばかりの頃は、どのようにレビューをすればいいのか本当にわかりませんでした…
今はコードレビューにもずいぶん慣れてきたので、新卒エンジニアの頃に感じていた
といったポイントを踏まえて、コードレビューをするときの4つの観点についてお話していきます。
なお、今回の記事はあくまでも新人エンジニアに向けてのコードレビュー観点を示すものです。ある程度レビューに慣れている方はまた別の視点があるということは最初に明記しておきます。
コードレビューを新人エンジニアに依頼する目的
新人で入って間もない頃だと「上司が書いたコードのレビューなんてできない!」と思いますよね。
自分よりも圧倒的に豊富な知識と経験があるため、指摘できる点なんてないと思ってしまうためです。
しかし、それでも新人のあなたにコードレビューを依頼するのは下記のような理由があります。
- コードレビューを通して新人エンジニアに学んでもらう
- ケアレスミスのチェックをしてもらう
- 読みにくい箇所を明らかにしてもらう
コードレビューを通して実装方法について学ぶ
当然ながら、新人エンジニアは上司よりも実装力がありません。そんなことは上司も分かっています。
なので、
とんちんかんなことを言ってしまったらどうしよう…
などと不安になることはありません。
むしろ、上司のソースコードを読むことで、新人エンジニアのスキルアップを図っているという側面があります。
他人のコードを読むことによって、
- きれいなコードの書き方
- 実装にあたっての設計
- フレームワークの知識や構成
などを学習することができます。
コードレビューをしてもらうと言いつつも、新人エンジニアに見て学習してもらう」という意図があることを忘れないようにしましょう。
ケアレスミスを減らせる
上司も当然人間なので、書いてあるコードにケアレスミスが混じっている可能性があります。
多くの場合、コードレビューに慣れてしまった数年目のエンジニアよりも、コードレビューの仕方がわからない新人エンジニアの方が時間をかけて丁寧にレビューをします。
上司からすると簡単なレビューであったとしても、こうしたミスを無くす可能性が上がるなら依頼しない手はありません。
なので、気になった点は「ここってミスですか?」と質問してみるといいでしょう。そんな小さな質問が指摘になり、技術的なレビューができるようになっていくのです。
読みにくいコードの箇所を明らかにする
新人エンジニアは経験を積んでいるエンジニアと比べると、どうしてもコードを読む力が弱いです。
しかし、読む力が弱いことはコードレビューにおいては強みになります。
なぜなら、読む力が弱いということは、コードの理解にあたって
- 理解できないコードは読みにくいコード
- 理解できるコードは読みやすいコード
と考えることができるためです(本当に理解力がないのはだめですが)。
上司からすると、自分が書き慣れている書き方をするため、そのコードが読みやすいかどうかの意識が新人よりも薄れがちです。
新人エンジニアが読めないコードの多くは、綺麗なコードではありません。読みにくいなと感じたら、それとなく「ここってどんな動作をしているのですか」と質問をするのもいいかと思います。
コードレビューの4つの観点
では、コードレビューを進めるにあたっての4つの観点を紹介していきます。
新人エンジニアが見るべきコードレビューの観点は、下記の4つです。
- コードがきちんと動作するか
- コードで読みにくい箇所がないか
- 変数やメソッドの命名が適切か
- 設計書と照らし合わせて違和感を感じないか
コードがきちんと動作するか
そもそも実装された部分が正しく動作するか確認する必要があります。具体的なチェックポイントは、
- 画面表示に違和感がないか
- 仕様通りに動作するか
- 仕様が不明瞭な部分は勝手な仕様で実装をしていないか
- 動作が重すぎることはないか
- 動作すべき環境すべてで問題なく動作するか
などです。
「確認されているから動作しないわけがない」と思われるかもしれませんが、テストにヌケモレがあり特定のケースでは動作しなかったパターンがたまに起こりえます。
特にフォーム周りは重点的にチェックしておくべきです。不具合があればサービス運営にクリティカルな影響がありますからね。varidationなどの細かいところまで確認するようにしましょう。
コードで読みにくい箇所がないか
上記でも触れましたが、理解しにくいコードは「自分の理解不足」か「本当に読みづらいコード」のいずれかです。
読みづらいコードを判別するポイントとしては、
- 不要なロジックやコード、コメントが混ざっていないか
- 条件分岐の多用などでコードが複雑になっていないか
- ネストは深くないか
- 重複したコードがないか
- インデントが整っているか
- 第三者が見た時に理解できるコメントになっているか
- 特殊なロジックが入っていてもコメントで説明がなされているか
などがあります。こういったことをチェックしておくとよいでしょう。
とはいえ、すべてを完全にチェックすることは難しいです。特に最初のうちは。
まずは自分が理解しにくかった箇所を指摘するだけでも、コードを改善するためには大きな意味を持ちます。なので、書いてあるコードについては、とにかく理解できるまで聞くという姿勢で臨みましょう。
変数やメソッドの命名が適切か
変数やメソッドがわかりやすい名前であるかも見ておきたいポイントです。具体的には、
- スペルは正しいか
- tmp, result などの汎用的すぎる変数を使っていないか
- メソッド名から返ってくる値が予測できるか
- コード内で名称が統一されているか
などを確認しましょう。
変数やメソッドについては、ある程度パターン化されており下記のQiitaなどは参考になるでしょう。
タイプミスのチェックもそうですが、「なんとなくこの変数に違和感があるな」と思って上記記事を調べてみると、実はもっと良い変数名が見つかったりします。
変数名やメソッド名の指摘も立派なコードレビューなので、気づいたときにどんどん指摘しましょう。
設計書と照らし合わせて違和感を感じないか
動作の確認を行う前に、そもそもどういう変更を加えたのかを把握するために設計書は見ると思います。しかし、設計書の仕様と動作が合っているかだけでなく、設計書とコードを見比べることにも価値があります。
最初のうちは、設計に基づいてコードが実装されているか分からないことも多いと思うので、コードのチェックというよりは、自分のスキル向上のためにやるという目的の方が大きいです。
コードレビューの際に設計に合ったコードを見ていると、今度自分がコードを書く際にも設計を意識したコーディングが徐々にできるようになります。
そうすると、より設計に詳しくなり、レビューを依頼されるときも「こっちのほうが設計方針に合っているのでは…?」というシーンが出てくるようになるのです。
特に新人エンジニアの間は、コードを読むことはコードを書くこと以上のスキルアップになります。どのような設計方針でどのようなコードが書かれているのか見て自分のスキルアップにつなげていきたいものですね。
コードレビューをするときに気をつけたいこと3つ
コードレビューの4つの観点に加えて、コードレビュー時に気をつけたいことを3つ紹介します。
- レビューされた人が対応しやすいコメントを返す
- できればレビューの根拠を示す
- 事前にチェックリストを用意する
少し補足します。
「レビューされた人が対応しやすいコメント」とは、例えば
- 端的で分かりやすいコメント
- 対応必須のもの(明らかな間違い)と、修正したほうがいいか分からないもの、理解しづらいものなどを分けておく
といった感じです。
そして、可能ならばレビューの根拠も示せたほうがいいですね。特にバグを見つけたときは、どういう条件でどういう動作を行った時に発生したのかは記載しましょう。
また、この記事などを参考に、事前にチェックリストを用意しておくとスムーズにコードレビューできます。そして、日々そのチェックリストを更新していけば楽ですし、後輩ができたときにも共有できます。
まとめ
コードレビューの観点について紹介してきました。この記事のポイントをまとめます。
- 新人エンジニアはコードレビューを通してコードの書き方を学ぶ
- コードレビューにおいては経験のなさといった弱みを強みに変える
- 新人エンジニアはコードを読むことが書くことよりもスキルアップに繋がる
今回はあくまでも新人エンジニア向けということで省きましたが、今回紹介した観点以外にもレビューをするにあたっての観点は数多くあります。
繰り返しですが、いろいろと試行錯誤をしていく中で「何をコードレビューの観点にしておくといいか」のチェックシートを自分の中で作っていきたいですね。