mixi Advent Calendar の17日目です。
今までまともにコードレビューをしたことがなかったエンジニアが、半年前からコードレビューを経験し始めて、チームの雰囲気が良くなった(と思っている)レビューの仕方をご紹介します。
私は株式会社ミクシィのグループ会社である株式会社 Diverse へ出向して、そこでデーティングサービス YYC の Android クライアントを開発しております。
現在、 YYC の Android クライアントは大規模な改修を行っていて、それに合わせて開発チームの再編成・開発フローの改善などいろいろな試行錯誤をしています。
その試行錯誤のうちの一つとしてコードレビューでの取り組みをご紹介します。
ここで説明すること、しないこと
ここでは以下のことについて説明します。
- コードレビューの雰囲気を良くする(良く保つ)方法
- 自分やチームメンバーの成長を促すための心がけ
また、以下のことはあまり説明しません。翌日の Diverse Advent Calendar で記事にしますのでそちらを参照してください。
- master のコードの質を上げるためのチェックポイント
- 自分のレベルを上げるコードリーディングの方法
なぜコメントの仕方が大事なのか
個人的に、コメントの仕方というのは重要だと思っています。
というのも、 Team Geek などにも書かれている通り、コードレビューは少なからず人に(精神的)ダメージを与えるためです。
自分が自信を持って(あるいは、苦労して)書き上げたコードに、他の人から指摘がなされるというのは、決して気分の良いものではありません。
私もそうでした。
自分の価値とコードを同一視してはいけない
(『Team Geek - Google のギークたちはいかにしてチームを作るのか』 2013 オライリー・ジャパン P.21)
このことを知っていれば大きくダメージは軽減できるものの、レビューで色々な要求されたり、なかなかマージしてもらえずにストレスが溜まることはどうしても避けられません。
そのため、なるべくダメージを軽減するような、ダメージを受けてもそれが学びになるような、そうしたコメントをすることが重要なのではないでしょうか。
コメントの方法
直して欲しいところにコメントする
レビューイーが提出したコードに対して、以下のように感じたら修正を要求することになるでしょう。
- そもそも動かない
- エラーが出る、クラッシュする
- Lint が警告を出している
- テストが通らない
- バグっている、も含む
- 明らかに非効率的
- 一見して何をしているのかわからない
プロダクト、そしてコードの品質を上げるためには指摘が必要です。
しかし、「ここが良くない」とだけ指摘にして Green さんの広告のように後の返事は「ググれ」では、チームの雰囲気は悪くなるばかりでしょう。
雰囲気を良くしながら指摘をするコメントとはどんなコメントなのでしょう。
事情がないなら、可能な限り代替案を添える
コメントには最低限以下の要素が含まれているべきです。
- どこを直して欲しいのか (Where)
- なぜ直して欲しいのか (Why)
それに加えて、 どう直したら良いのか (How) を添えると良いでしょう。
「このクラス名 ViewHolder
は何の ViewHoler なのか漠然としているので、 BbsEntryViewHolder
とかにするともっとわかりやすいと思います!」
ただ指摘するより、「キチンと考えている」親身な感じが出るかと思います。
(新人育成などでレビューイーに考えてもらう必要がある場合はその限りではありませんが)
「マズイのはわかるんだけどどう直したらいいのかわからない!」
というときには素直に言いましょう。
「ここ、引数から null が入る可能性があるので NullPointerException が起きそうです…! いいアイデアが無いのですが、どうしたら直せますでしょうか?」
メンテナビリティについて指摘する場合は、まずは「動く、すごい!」というスタンスでコメントすると良いように感じます。
「このメンバ mInnerView
はローカル変数にしてしまうと、コードが読みやすくなってもっと良くなると思いますー!」
そして、修正していただいたら、なるべくまたコメントをするようにしています。
テンション高く、ありがたさが伝わると良いと思います。
あくまで直してくれた人のおかげでコードが良くなったことを肝に銘じるようにしています。指摘した人は横から口を出しただけ、のスタンス。
「おおお、フローがもっとわかりやすく!! すごいですー! 」
良い書き方だと思ったところにコメントする
指摘をするだけではなく、以下のような箇所もコメントするようにしています。
- 初めて知ったこと
- 文法、メソッド、ライブラリ
- テクニック
- など
- その発想はなかった・うまい!
- 設計
- 命名
- など
- 以前から上達しているところ
- など
「こんな書き方があるんですね!! 勉強になりますー!」
「これは便利なビュー…!! こっちの画面でも使わせていただきます!」
「いつもドキュメントコメントがすっきりかつわかりやすくてすごいです…!」
褒められて悪い気分になる人はあまりいないと思いますので、ガンガン褒めるスタンスで良いと思います!
私も褒められたい⁽⁽٩(°ꇴ°⑉)۶⁾⁾
そんなレビューを続けた結果
現在のプロジェクト発足前は、距離感がわからないのもあったのか、かなり何でもLGTMが出ていた印象でした。
しかし、プロジェクト発足と同時に、上記のようなレビューを始めて3か月続けた結果。
穏やかに意見交換が起こりつつ、良い雰囲気で開発が続けられているように感じます。
また、チーム全員のコードの書き方も改善されてきたようで、指摘コメントの数が段々減ってきているようです。
(ムラがあるので一概に減ったとは言い切れないのですが、感覚的には「簡単なことを簡単に書けるようになってきた」と感じています)
全く別チームのディレクターさんがコードレビューの様子を見てくださっていて、
「コメントのやりとりが楽しそうで、いい雰囲気ですね」
と言ってくださったので、他の方の目から見ても良い雰囲気なのかなと思っています。
次の目標は、この雰囲気をそのままに活発に意見交換を起こすことです。
もっと品質の良いコードを残せるようにしてゆきたいです。
以上、良い雰囲気で言うべきことが言えるコードレビューでのコメントの仕方でした。
明日は @kubo39 が、 @tanatana さんみたいに面白いことを書いてくれると思います。