2019.01.18

ソースコードレビュー力をあげる

shimo

いかに良いコードを書くかという本やドキュメントは膨大にありますが、いかに上手にコードレビューをするかについては比較的ドキュメント等が少ないと感じています。また、コードレビューは最終的なコメントについては形として見えますが、コードレビューをしているメンバーが頭の中でどういうことを考えているのかというところは見えにくいという部分もあります。

以前、ソースコードレビューについては、

という記事を書きましたが、今回はコードレビュー力を上げるためにはどうしたらよいか、主に以下3つの観点から書いてみたいと思います。

スキルアップしよう！
レビューイーによって観点を変えよう！
楽しく積極的にコードレビューに参加しよう！

スキルアップしよう！

コードレビューに必要なスキルとして、大きく

コードリーディング力
プロダクトナレッジ

の2つを挙げたいと思います。

Contents

コードリーディング力
1. その言語をある程度知る
2. とにかくたくさん読むことに慣れる
プロダクトナレッジ
1. 開発対象のプロダクトの機能・実装を把握する
レビュー前に実装を想像する
楽しくレビューする
わからないことは聞く

コードリーディング力

効率よく良いコードレビューをするには、コードリーディング力は必須です。

その言語をある程度知る

大前提として、レビュー対象の言語を知っている必要がありますが、パラダイムが同じような言語であればなんとなく理解できると思いますので、レビュー観点にもよりますが実は場合によっては必須ではありません。ただ、細かい部分をレビューする必要がある場合は、その言語に対する知識は必須となるでしょう。また、例えば関数型言語を知らない人がいきなりHaskellのレビューに参加などはなかなかつらいと思います。

とにかくたくさん読むことに慣れる

色々なソースコードを読みましょう。今はいい時代で、たくさんオープンソースのコードがありますので、お好きなものを読みましょう。
コードリーディングの経験を積むにしたがって、

こういう機能だったらこういう実装になってるかなー
データ構造やメソッド・変数の命名など名前の付け方のクセ

など想像しやすくなってくると思います。

プロダクトナレッジ

開発対象のプロダクトの機能・実装を把握する

以前、コードレビューは差分以外を読め！という記事を書きました。

実装の必要性および正当性をチェックするのもレビューの重要な役割ですが、十分性をチェックするところにコードレビューのさらに重要な意味・真髄があると思っています。そして、実装の十分性をレビューするためには、まず機能の全体像を把握しておくことが重要です。

また、仮に途中から参加したプロジェクトでも既存コードを読んで実装を把握しておくのも大事なことです。
昨年末に私の個人ブログでさらっと触れたのですが、以前の職場での100万行ほどのソースコードがある中で、新入社員だった私は既存機能やコードをそこまで把握しないまま機能追加実装をしてしまいました。もちろん関連しそうなコードは読んだつもりでしたし、新卒でいきなり100万行のソースコードを全部把握するのは無理だったというのは言い訳です、すみません。ただその結果、多くの考慮漏れが発生しました。まあ当たり前の結果です。

レビューイーによって観点を変えよう！

効率的にレビューするには、レビューイーのスキルとプロダクトナレッジによって、レビュー観点を変えるのが良いと思います。非常に簡略化したケースとなりますが、以下のような感じです。

レビューイーのスキルが高く、プロダクトナレッジも豊富にある場合
実装されている部分についてはおそらく問題ないだろうと信頼した上で、細かい実装については斜め読みし、時間節約します。重要ロジックであればきちんと見ます。テストコードも斜め読みです。ケースが不足してないかぐらいしかみないことも多いです。
レビューイーのスキルが高いが、プロダクトナレッジが少ない場合
レビューのしがいがあるパターンです。細かい部分は見ませんが、ロジックの正当性、そして特に十分性チェックに力を入れてレビューします。テストコードもテストケースについてしっかりとみます。
レビューイーのスキルが低いが、プロダクトナレッジが豊富にある場合
このケースは少ないので書きません。
レビューイーのスキルが低く、プロダクトナレッジも少ない場合
これもレビューのしがいがあるパターンです。あらゆる観点からレビューします。

楽しく主体的にコードレビューに参加しよう！

レビュー前に実装を想像する

主体的にコードレビューに参加する上で、オンラインレビューであればレビュー前に準備をしましょう。

これは人によってやり方が分かれるとは思いますが、私の場合以下のような準備をすることが多いです。

機能要件を把握した後、実際にコードを見る前に、修正すべき箇所および実装イメージを事前に考えておきます。大事なのは自分がプロダクトナレッジを持っている場合、修正すべき箇所を洗い出しておくことで、これは比較的時間をかけて考えます。

そして実際にコードを見ると、事前に考えた実装イメージより、実際の実装は複雑になっているケースの方が多いと思います。したがって、逆にイメージと合っているような場合や、イメージより簡単な実装になっている場合は考慮不足がないかを疑うことになります。

楽しくレビューする

レビューア、レビューイーともに楽しくレビューしあえるのが理想だと思います。コードレビューの場が厳しすぎたり殺伐としているとレビューイーは萎縮してしまい、過度に完璧を求めて失敗を恐れるようになる結果、開発スピードが遅くなったりします。
オンライン・オフラインいずれにせよレビューの場はお互いの成長の機会ととらえ、楽しくありたいものです。 お互いの成長の機会 です。仮にレビューイーが新人だったとしてレビューアとのスキルの差が非常に大きい場合でも、レビューアにとっても成長の機会にもなります。例えば、どう伝えるのが一番いいかを考えたり etc。。。

わからないことは聞く

聞くは一時の恥、聞かずは一生の恥 ということわざがありますが、私はこのことわざがあまり好きではありません。もちろん大人の事情で聞けないようなケースもなくはないですが、基本的に聞くことは恥ではないと思うからです。
前述した通り、コードレビューは品質向上のためのプロセスである一方、勉強の場でもあります。聞かなければ、せっかく学習するチャンスを逃してしまうことになります。
聞かれた方も、なんでそんな簡単なこと聞くんだと思わないようにしましょう。自戒もこめて。。。

ただし、私自身、質問して説明されても理解できないこともたまにあります。通常は理解できるまで聞くようにしていますが、やむを得ずどうしても時間がないような場合は、自分がまだ理解していないことを伝え、今後また聞くかもしれない旨を伝えます。