Facebookでこのスライドが紹介されていたのを見かけて、レビューについて個人的に思うことがあったので書いておきたいと思います。
私自身がレビューをするときは、
「指摘した箇所の代替案を出す」
というのを必ずやるようにしています。
「この書き方だとXXXだからよくない」
と指摘した場合に、指摘された箇所がよくないことは指摘された本人もわかっているケースが多いです。
自分の経験では、新人の人ですら指摘内容について「僕もそう思ったんですけど・・・」とつぶやくのをよく見たことがあります。
レビューされる側が自分のコードが良くないということに気付いているというのはよくあることだと思います。
良くないのはわかっているけど、じゃあどうするのがベスト・プラクティスか分からないという状況ですね。
被レビュー者がどう実装すべきか悩んだ結果をレビューしているのだから、
- 「自分ならこうするよ」
- 「一般的にはこういったアプローチがあるよ」
- 「前のプロジェクトではこうやってうまくいったよ」
というノウハウを代替案(助け舟)として伝えてあげて、被レビュー者の成長を後押ししてあげるのが理想的なレビューだと思います。
~コードは誰のものか~
レビューのもう一つの効果は、
コードを「個人の品質」から「組織の品質」に変えること
にあると思います。
レビューしていないコードでバグが出た場合、コードを書いた人が責められ、
「てめぇ何やってんだ!今日中に直せゴラァ!」
みたいな光景がしばしば展開されます。
一方、レビューをしたコードでバグが出た場合、コードを書いた人は
「待ってください!レビューのとき、誰も指摘しませんでしたよね?」
「みんなが気付けなかった問題を、私の責任にするのですか?」
と主張することができます。
一見開き直りのように見えますが、オープンソースや趣味のプログラミングでない限り、通常コードは自社の製品か受託あるいは派遣といった業務で書かれます。
※複数人で開発されるオープンソースには、コードに統一性がなくてぐちゃぐちゃしている場合がありますが、各人それぞれの趣味を優先しているからだとおもいます。
コードを含む成果物は組織に属するものであって、個人に属する物ではありません。
従って、不具合の責任を個人に押し付けること自体本来おかしなことです。
ソースコードはほっておくと属人性が高くなりますが、コードレビューにはそれを防ぐための対策としての意味があると思います。
そうは言っても忙しくてレビューなんて真面目にやってられないよ
実際の現場でのコードレビューは時間をかけてやる割にあまり意味のないレビューも多いです。
レビューは各人が本来の業務をやっている最中に
「XXXのレビューしてください」
という依頼が割り込みで入ってくることが多いと思います。
このときレビューする人が、
「俺忙しいし、適当に見て、自分の仕事に戻ろう」
「自分の担当外だし、どうでもいいや」
という意識でレビューしても意味のあるレビュー指摘は出ません。
忙しい現場だと、こうして大した指摘もせずにレビュー工数だけカウントして実質的に意味のないレビューをしてしまいがちです。
会議で発言しない人の存在がよくに問題になりますが、レビューも同じで「レビューで指摘しない」というもの同じように問題だと思います。
レビューで指摘がでないということは、
- 真面目にレビューしていない
- 指摘者のスキルがたりない
- 指摘する箇所がない完璧なコードが書かれている
の3種類があると思います。
※現実には1と2しかないということはみなさんご理解頂けるかと思います。(笑)
そして、1と2はいづれもレビューする側の問題です。
レビューする側にも責任があることを意識した上でレビューをすると、レビューの時間の密度が濃くなり、1回1回のレビューがやるかやられるかの真剣勝負のようになります。
レビュー側も指摘するために勉強しなければなりません。
そうなると、レビューはレビューする側にとっても、される側にとっても、お互いに切磋琢磨できる時間になり、自ずとコードの品質は向上すると思います。
なにより、ソースコードの属人性を薄め、品質を組織としての品質に変えることができるようになると思います。