人間らしくコードレビューするには(パート1)

最近、コードレビューのベストプラクティスについての記事をいくつか読みました。これらの記事はバグを見つけることにフォーカスしていて、レビューにおける他の要素には焦点があたっていないことに気が付きました。見つけた問題点に対して建設的、かつうまくコミュニケーションをとる方法？全然違います！バグを全て抽出し、あとはよろしくとなっています。

そこで、私はお告げを受けました: これがもしコードでうまくいったら、なぜロマンスがうまれないといえましょうか。 私は開発者の皆様に愛が芽生える新しい電子書籍をだそうと思います。

私の革新的な電子書籍はあなたのパートナーの不備を最も多く見つける 実証済みのテクニックをご案内します。この本は次のことは取り扱いません

• 発生した問題に対してパートナーと相手の感情を理解してコミュニケーションする
• あなたのパートナーの弱みを指摘しやすくする

私がコードレビューの書籍を読んだ限りでは、人間関係については、明白であり議論するに値しません。

この電子書籍はお役に立ちそうでしょうか？きっと「違う違う！」と甲高い声でいわれるのではないかと思います。

それでは、なぜそのようになってしまったのでしょうか？

私は、今まで私が読んだ書籍は未来の書籍であり、そこでは開発者は全てロボットになっているとしか考えられません。その世界では、チームメイトはコードに対する思慮のない言葉での批評で、その冷たいロボットの心を暖かくし、これを喜んで受け入れるでしょう。

私はあなたがいま現在のコードレビューを改善したいという大胆な仮説をたてようとしており、そこではあなたのチームメイトは人間です。さらに大胆な仮説として、同僚と良い関係となれば、これはただ単に各不備を改修するコスト(cost-per-defect)を最小化するだけでなく、結果的に以前よりより良い関係が築けるとしましょう。このような環境下でどのようにあなたのレビューを改善できるでしょうか？

この記事では、コードレビューに関しての、技術的なプロセスだけではなく、同時に良い関係が築ける技術について議論したいと思います。

コードレビューとは何か？

「コードレビュー」という言葉は、チームメイトの肩越しにコードを読むことから、20人の打合せでコードを1行ずつ解析することまで様々な活動で使われます。私はこの用語を、打合せで直接コードインスペクションするような大掛かりなものではなく、形式的に書かれたものとして進めるものとして利用します。

コードレビューの参加者としては、コードを書いてレビューを依頼するオーサーと、コードを読みチームのコードベースにマージできるかを判断するレビュアーがいます。レビューは複数のレビュアーを設け実施することもできますが、単純にするためにあなたが唯一のレビュアーであると仮定することにします。

コードレビューを開始する前に、オーサーは変更リスト(changelist)を作成しなければなりません。コードレビューは複数回のラウンドで実施されます。それぞれのラウンドはオーサーとレビュアーの間の1回の往復で完了します。オーサーは変更を提示し、レビュアーはこれらの変更に対してフィードバックを書き起こしそれに応えます。コードレビューは1回以上のラウンドとなります。

レビューはレビュアーが変更を承認したら完了します。これは通常LGTM(「looks good to me」の短縮形)をというコメントをつけることで行われます。

なぜコードレビューは難しいのか？

プログラマーが彼らが最高と考えている変更をあなたに提示し、あなたがなぜそれが素晴らしくないかを大掛かりに一覧を書き起こす必要がある場合、これを伝えるのはとてもセンシティブなメッセージです。

私がITに戻りたいと思わない理由の1つが、プログラマーの人々はとても好かれない人々であることです... 例えば航空業界では自身のスキルレベルを大きく過信した人はみな死んでしまうのです。

Founder at WorkからArsDigitaの共同創設者であるPhilip Greenspun氏の言葉の引用

オーサーは彼らのコードへの批評を、彼らが能力のないプログラマーであると暗に伝えているととらえるのは容易でしょう。コードレビューは知識を共有し、より確かな知識に基づき技術観点の決断をする機会です。しかし、オーサーがこのディスカッションを個人への攻撃と認識すればそれらは達成されません。

あなたの考えを書いて伝えることは、そんなに難しくないと思われるかもしれませんが大変なことです。ミスコミュニケーションのリスクはより高くなります。オーサーはあなたの声の調子もわかりませんし、ボディーランゲージもわかりませんので、注意深くはっきりとフィードバックすることがより重要となります。批評に対して身構えているオーサーからすると、「ファイルハンドラーをクローズし忘れています」といった何の変哲のないコメントでも「ファイルハンドラーをクローズし忘れるなんて信じられませんね！あなたは本当に間抜けですね」といったように解釈しうるのです。

コードレビューのテクニック

• 1. 単純なことはコンピューターにやらせてしまう
• 2. コードの記法については規約で明確に決める
• 3. すぐにレビューを始める
• 4. 高位のレベルから始め詳細化する
• 5. コードのサンプルを示して寛大に
• 6. 相手に直接言及しない
• 7. フィードバックする際に命令するのではなく、お願いする
• 8. 意見ではなく原則に基づいてコメントをする

単純なことはコンピューターにやらせてしまう

打ち合わせやEメールのような割込みがあり、あなたがコードにあてられる時間は多くありません。そして、精神的にも持続しないものです。チームメイトのコードを読むのは経験的にとても骨が折れることで、高レベルの集中が求められます。これらのリソースをコンピュータができること、特に人間よりコンピューターのほうがよくできることに無駄遣いしてはなりません。

空白のエラーはわかりやすい例です。人間のレビュアーがインデントのミスを見つけ、修正するためにオーサーとやり取りするのと、自動書式整形ツールを利用するのとを比べてみましょう。

人間のレビュアーが実施する際に必要なコスト

• 1. レビュアーが空白の問題や誤ったインデントを探す
• 2. レビュアーが誤ったインデントについてコメントをかく
• 3. レビュアーがコメントが明確で、非難するような書き方をしていないことを読み直して確認する
• 4. オーサーがコメントを読む
• 5. オーサーがコードのインデントを修正する
• 6. オーサーがコメントに対して適切に対処したことをレビューワーが確認する

書式整形ツールを使う際に必要なコスト

なし!

後者の書式整形ツールを使った場合は、オーサーが「保存」ボタンを押すたびに空白を自動で直してくれるツールを利用しているため、なにもコストがありません。最悪のケースでも、オーサーはコードに対するレビュー依頼を実施し、継続的インテグレーションによって空白が正しくないことを知らせてくれます。レビュアーが全く気にすることなく、オーサーは問題を修正します。

自動化することによってレビューから取り除ける機械的にできるタスクについて見てみましょう。ここに一般的なものを記します。

自動化することによりあなたはレビュアーとしてより価値のある貢献ができるようになります。importsの順序やソースコードのファイル名の命名規約などの問題を無視できるならば、機能上の誤りや可読性の問題といった、より関心のある問題にフォーカスすることができます。

オーサーもまた自動化の恩恵を受けます。ケアレスミスを見つけるのに1時間浪費することなく、即座に見つけられます。即座にフィードバックを受けられることで、関係のあることがオーサーの頭に残り、これにより学習が容易となり、修正コストが低くなります。それに加え、彼らが初歩的な誤りについて指摘を受ける必要がある場合、あなたから指摘を受けるよりコンピューターから指摘を受けたほうが彼らの自尊心の観点からはるかに気分がよいわけです。

これらの自動チェックはコードレビューのワークフローの中に入れましょう(例えば、Gitのコミット前のフックやGithubのwebhookです)。オーサーがこのチェックを手動で行うことを前提としておけば、あなたはこの恩恵に授かれます。 オーサーもたまに忘れることがありレビュアーがやむをえずこの単純な問題をレビューし続けなければいけない場合も、自動化すれば代わりに対処してくれます。

コードスタイルについてはコーディング規約で明確に確定させる

レビューする際のコードスタイルについてのコメントは時間の無駄です。一貫性のあるスタイルはたしかに重要ですが、コードレビューは波括弧 { をどこに書くかについて言い争う場ではありません。書き方についての議論をレビューから切り離す最善の方法はコーディング規約を最新に保つことです。

良いコーディング規約は、命名規約や空白のルールのような表面的なことだけでなく、そのプログラミング言語の機能をどのように利用するかについても定義します。例えば、JavaScriptとPerlでは、たくさんの機能が提供されており、同じロジックに複数の実装があります。コーディング規約は、チームの半分がある言語の機能群を使い、残りの半分が違う機能群を使うようにならないようにならないよう、「唯一の正しい方法」を定義します。

一度コーディング規約を決めれば、どの命名規則が最善かについてオーサーと議論し、レビューのサイクルを無駄にする必要がなくなるでしょう。コーディング規約に従うだけで次に進めます。コーディング規約にある問題に対して規定がない場合、一般的にはこれは指摘するに値しません。コーディング規約でカバーしていない問題に直面し、それが議論に値することだとすれば、チームで詳細に議論しましょう。それから、この議論を2度としなくてよいようにコーディング規約に決定したことを追記するのです。

選択肢1: 既存のコーディング規約を採用する

インターネット上を検索すれば採用するのにふさわしい公開されたコーディング規約を見つけることができるでしょう。Googleのコーディング規約が最もよく知られたもので、仮にこれがあなたに合わなければ他の規約を見つけることができるでしょう。既存のコーディング規約を採用することで、1から作成するコストを払うことなくコーディング規約の恩恵にあやかることができます。

否定的側面としては、既存のコーディング規約は彼ら自身の要望に合わせて最適化されていることがあげられます。例えばGoogleのコーディング規約は新しい言語の機能を利用することには保守的であり、これは彼らが自宅用のルーターから最新のiPhoneまで動作させる必要のある膨大なコードベースを持っているためです。あなたが4人である1つの製品を作り事業を立ち上げているのならば、もっと最新の言語の機能や拡張機能を積極的に利用することを選択することでしょう。

選択肢2: コーディング規約を少しずつ増やしていく

既存のコーディング規約を採用したくなければ、自分で作ればよいでしょう。コードレビューで規約に関する議論が発生した場合、チームとしてどのようにすべきかを決めるためにチーム全体に問いかけましょう。合意したら、コーディング規約に追記しましょう。

私はチームのコーディング規約をMarkdown形式でバージョン管理下に置くのが良いと思っています。(例えば、GitHub pages)。そのようにしておけば、コーディング規約のいかなる変更も通常のレビュープロセスを経ることができます。つまり、だれかが変更を明示的に承認する必要があり、チームの全員が意見を述べるチャンスがあります。WikiとGoogle Docsも選択肢としてはよいでしょう。

選択肢3: これらを組み合わせる

選択肢1と選択肢2を組み合わせることで、既存のコーディング規約をベースとし、これを拡張またはオーバーライドするためのチーム固有のコーディング規約を維持できます。Chromium C++コーディング規約がこれのよい例です。GoogleのC++コーディング規約をベースとして、独自の変更や追加をしています。

すぐにレビューを始める

コードレビューを最高優先度にしましょう。時間を取って、コード実際に読んで、フィードバックする際に、レビューをすぐに開始しましょう。理想的には数分以内に、です。

チームメイトが変更リストを提示している場合、あなたのレビューが終わるまで彼らの仕事がブロックされていることがありがちです。理論上は、ソースコードのバージョン管理システムはオーサーが、ブランチを作成することを許容しており、仕事を継続し、レビューに対する変更を彼らのブランチに前方マージします。現実にはこれを効率的にできる開発者はほぼいないのです。レビューが返ってくるまでに待っている間の変更を考慮して3ウェイマージの差分を紐解くのに大多数の開発者に長い時間がかかります。

レビューをすぐに始めれば、好循環を作ることができます。レビューが返るまでの時間が単純にオーサーの変更の複雑さと機能の大きさで決まります。これはオーサーが、小さく、スコープを区切った変更リストを提示する動機付けとなります。これらはあなたがレビューする際により容易になり嬉しいはずで、レビューが高速化し、良いサイクルが継続します。

あなたのチームメイトが1000行のコードの変更を含む新機能を実装したシーンを想像してみてください。もし彼らがあなたは200行の変更リストを2時間でレビューできるとわかっていれば、彼らはおよそ200行毎に変更リストを分割し、全機能を1日か2日でチェックしてもらうことができるでしょう。しかしながら、その変更の規模にかかわらず、全てのコードのレビューに丸1日かかっている場合、その機能をすべてチェックするには1週間かかるでしょう。あなたのチームメイトは1週間にわたって座っていたくないですから、500行から600行といった大きなコードレビューを送る動機が生まれるでしょう。200行の変更より600行の変更のほうが内容を覚えていることが困難になりますので、レビューにより多くのコストがかかる一方でフィードバックの内容が乏しいものになります。

レビューの1ラウンドは最大で1営業日であるべきです。もしより優先度の高い問題に取り組む必要があり、レビューを1日以内に完了できない場合は、チームメイトにこれを知らせ、他のメンバに再アサインできるようにしましょう。レビューを1ヶ月に1度以上辞退せざるをえない場合は、あなたが健全な開発プロセスを維持できるようチームはペースを落とす必要があることを意味します。

高位のレベルから始め詳細化する

1回のレビューラウンドで多くのコメントをつければつけるほど、オーサーが困惑するリスクが増します。この上限は開発者によって異なりますが、1回のレビューのラウンドで20から50のコメントがつくと危険ゾーンです。

オーサーがコメントの海に溺れてしまうのが心配であれば、初期のラウンドでは高位のフィードバックを行うように自制しましょう。クラスのインターフェースの再設計や、複雑な関数の分割といった問題に集中しましょう。この問題が解決するまで待ってから、変数名のつけかたや、コードのコメントが明瞭かといった低位の問題に取り組みましょう。

低位のコメントについてはオーサーが高位のコメントを反映すると無意味になります。これの低位のコメントを後のラウンドに遅らせることで、あなたが問題を指摘するために注意深くコメントを記入する馬鹿にならない時間を節約できますし、オーサーが不要なコメントに対応しなくてよいようになります。このテクニックはレビューの最中に抽象レイヤーを分割し、あなたとオーサーが変更リストを明確に、システマチックに処理することに役立ちます。

コードのサンプルを示して寛大に

理想的な世界では、コードのオーサーは彼らが受け取る全てのレビューに感謝します。レビューは学びをえる機会であり、誤りを防ぐものです。現実世界では、オーサーがレビューを否定的に認知しコメントを付けたことに腹立たしく思う外的要因がたくさんあります。期限を守るためにプレッシャーを感じており、あなたの即座の、承認の印以外は邪魔だ、と感じるかもしれません。そんなに一緒に仕事をしていないため、あなたのフィードバックが善意であると信じられないかもしれません。

レビューのプロセスの中でオーサーが気持ちよくなるための良い方法は、レビューの最中に彼らに贈り物を送るチャンスを見つけることです。開発者はどのような贈り物を喜んで受け取るでしょうか？もちろん、コードのサンプルです。

あなたの考えている変更のいくつかを書きおこしてあげることでオーサーの負荷を軽減すれば、レビュアーとしての寛大さが示せるでしょう。

例えば、同僚にPythonの機能のlist内包表記に慣れていない同僚がいたとしましょう。彼らは次のようなコードを提示しています。

urls = []
for path in paths:
  url = 'https://'
  url += domain
  url += path
  urls.append(url)

「リスト内包表記を利用してこれをシンプルにできませんか？」と答えた場合、彼らは今までに経験していないことを調べるのに20分程度調べて時間を使うのに苛立つことでしょう。

次のように書いてあげるとより嬉しくなるでしょう

次のようにリスト内包表記を利用してシンプル化することを検討してみてください。

urls = ['https://' + domain + path for path in paths]

このテクニックはワンライナーに限ったものではありません。私は、大きな関数を分割したり、追加のエッジケースをカバーするための単体テストを追加するなどといったときに、オーサーに大きな概念(proof of concept)を実証するために自分のブランチを作成します。

このテクニックは、明確に、議論の余地がなく改善するものにする必要があります。上記のリスト内包表記の例では、83%のコード削減に反対する技術者はほとんどいません。反対に、自身の好み(例えばスタイルの変更)に基づいて「より良い」長い変更を例示した場合、そのコード例は寛大であるというより押し付けがましいと見えるようになります。

1回のレビューラウンドでは2つか3つのコード例の提示に抑えましょう。オーサーの全ての変更リストに記載を始めた場合は、彼らがコードをかく能力がないと考えていることを示します。

相手に直接言及しない(Never say "you")

これは奇妙に聞こえるかもしれませんが、ちょっとよく聞いてください。コードレビューで「あなた」という言葉を使ってはなりません。

レビューで至った決断については、誰がそのアイデアを考えついたかではなく、どうしたらコードが良くなるか、に基づくべきです。チームメイトは変更リストに膨大な時間をかけ、やったことに誇りを持っています。彼らの成果に対する批評に対しては防衛的になるのが自然な反応でしょう。

チームメイトが身構えるリスクを最小化するようにフィードバックの言葉を選びましょう。コードを書いている人間を批評しているのではなく、コードを批評していることを明確にしましょう。「あなた(You)」という言葉がコメントにあると、これはフォーカスがコードではなく彼ら自身に移ってしまいます。これは彼らが批評を個人的なものであると捉えるリスクを増しています。

次のような害のないコメントについて考えてみましょう。

You misspelled ‘successfully.’

('successfully'の綴りが誤っています)

オーサーは2つの違った解釈をしえます。

• 解釈1: Hey、相棒よ！君は「successfully」のスペルを間違えてました。あなたは賢明だと思ってますよ！おそらく単なるtypoでしょう。
• 解釈2: 君は「successfully」のスペルを間違えてました。大ばかもの。

「あなた(you)」を削除してみましょう

sucessfully -> successfully

後者のコメントは単なる修正でオーサーの評価ではありません。 幸い、「あなた」という言葉を使わずにフィードバックを簡単に書き換えられます。

選択肢1: 「あなた(you)」を「我々(we)」に置き換える

Can you rename this variable to something more descriptive, like seconds_remaining?

(この変数をもっとわかりやすく例えばseconds_remainingに変えられる？)

これは

Can we rename this variable to something more descriptive, like seconds_remaining?

(変数名をもっと分かりやすくなるようにseconds_remainingとしてはいかがでしょうか？)

「我々(we)」という言葉はコードに対するチームの共同体としての責任であることを強調します。オーサーは別の会社に移動するかもしれませんし、あなたがそうなるかもしれません、しかし、このコードを持つチームはある形を維持するわけです。オーサーにしてほしいことを「我々は」と言うのは馬鹿馬鹿しいようにもみえますが、非難となるよりはましです。

選択肢2: 文から主語を取り除く

「あなた」を使わないようにするその他の方法として、文から主語を省略する方法があります。

seconds_remainingといったように、もう少しわかりすくことをおすすめします。

受動態を使うことによって同様の効果をえられます。私は技術文書で受動態を利用するのは一般的には避けるようにしてますが、「あなた」の周りに利用するのは有用です。

seconds_remainingといったように、もう少しわかりやすいように変数は名称を変更されるべきです。

「〜はどうでしょう？」といった疑問形式にしてみる選択肢もあります。

seconds_remainingとうようにもう少しわかりやすい変数名としてはどうでしょうか？

フィードバックする際に命令するのではなく、お願いする

コードレビューは、ディスカッションが個人への言及に脱線するリスクが高く、通常のコミュニケーションより気配りと配慮が必要です。レビュアーはレビューの際に丁寧であることを期待しますが、突飛なことで変な方向に向かってしまうことがあります。多くの人々は、同じ職場の人に、「そのホッチキスをとって、それからソーダを取ってきてください」とは決して言いません。しかし多くのレビュアーが「このクラスを他のファイルに移動してください」といった厚かましい言い方をしているのをみてきました。

フィードバックの際にはしつこいくらい丁寧にしましょう。コメントする際に命令するのではなく、お願いする、あるいはおすすめしてみてください。

次の2つのコメントを比べてみてください。

人は自分でコントロールできている状態を好みます。オーサーにお願いをすると自主性を持った感覚になります。

お願いはオーサーを礼儀正しい状態に戻しやすくなります。おそらく彼らは適切な判断をするでしょう。命令形でフィードバックすると、オーサーからの意見は反発しているように受け取られます。もしお願いあるいは疑問形式でフィードバックすれば、オーサーは単に答えれば良いことになります。

レビュアーが最初のコメントをどのようにつけるかによって、どんなに議論が喧嘩腰に見えるかを比べてみましょう。

命令する代わりにお願いする形でコメントすることで、どれだけやりとりに棘がなくなったかをご覧ください。

意見ではなく原則に基づいてコメントをする

オーサーにコメントを行う場合は、どのような変更が必要か、と変更の理由の両者を説明する必要があります。「このクラスを2つに分割してください」というのではなく、「現在のところ、このクラスはファイルのダウンロードとパースの両方の役割を担っています。これは単一責任の原則に基づいてダウンローダークラスとパーシングクラスに分割すべきです」という方が良いでしょう。

コメントをある原則に基づいて実施すると議論が建設的に進みます。次のような理由を引用した場合「クラスのpublicインターフェースを最小化するために関数をprivateにするべきです」、オーサーは単に「いいえ、私は私のやり方のほうが好みです」と答えることができません。実際には答えられますが、あなたが変更のゴールを述べているのに、好みについて述べたら馬鹿みたいに見えるでしょう。

ソフトウェア開発は芸術であり科学です。あなたは常に確立された原則に基づいて何が良くないかはっきりと正確に示せるわけではありません。コードが汚かったり直感的でなかったりするだけのこともありますし、これはなぜかを突き止めるのは困難です。このような場合には、客観性を保ち、あなたは何ができてるかを説明してみてください。「これは私には理解できません」というのは「これは混乱を招きます」という全ての人間に当てはまるかわからないものであるのに対し、少なくとも客観的な発言です。

リンクで論拠が補える場合は合わせて記載しましょう。チームのコーディング規約の該当の節へのリンクが一番実践しやすいものです。言語やライブラリのドキュメントへのリンクも利用できます。投票数の多いStackOverflowの回答も有効ですが、権威あるドキュメントから離れれば離れるほど、エビデンスの信頼性は低くなります。

パート2: もうすぐです.

2, 3週間以内に残りの半分の記事を公開する予定です。次のポイントを含む記事をお見逃しなく。

• あまりに大きなコードレビューに対応する
• 賞賛をつけるポイント
• コードレビューのスコープに気をつける
• 行き詰った状態を和らげる方法

編集: Samantha Mason、イラスト: Loraine Yow。この投稿の初期の頃から貴重なフィードバックをくれた @global4gに感謝します。