eityansメモ

eityansメモ

ゆるくやっていきます

記事一覧RSS

コードレビューで安易に質問形式を使うとチームが疲弊する

修正依頼の意図があるならば、素直にその内容を書いたほうが良い。安易に質問形式を使うとチームが疲弊する。


質問形式のレビューとは

例えばユーザーのidを抽出したいときに以下のコードがあったとして

1user_ids = User.where(hoge).map(&:id)

それに対してレビュアーが

map(&:id)を使っているのはなぜですか?

みたいな質問形式。これ、なるべくやめたほうが良い。

「この処理で必要なのはIDだけなので、pluck(:id)を使う方が効率が良いからそうしてほしい」という意図があるならば、シンプルにそう伝えれば良い。

上記の例は極端かもしれないが、以下のような書き方をしている人もいるのではないだろうか

pluckではなくてmapを使っているのはなぜですか?

退会ユーザーの場合はどうなりますか?

質問形式によるレビューの「こじれ」

質問形式を取ってしまうと以下のような「こじれ」が発生する

  • 真意を探る

    • 特に「なぜ?」と問われた場合に発生しがち。レビュイーからすると、レビュアーが何を問題視しているのか、どのような意図で質問しているのかを推測する必要が出てくる。「もっと良い方法があるのか?」「何か設計上の意図を確認したいのか?」など、レビュアーの意図が不明なため、レビュイーは様々な可能性を考慮し、本来のタスクから意識をそらすことになる。

  • 防御的な姿勢を取る

    • レビューの仕方や人にもよるが、自分の書いたコードを否定されたように感じ、防御的な姿勢を取ってしまう場合がある。

  • 無益な議論へと発展する

    • レビュイーが意図を汲み取れなかったり(大抵はできない)した場合に発生する。レビュアーからすると的外れな表面的な回答をレビュイーが行い、それに対してレビュアーがまた質問するという不毛な議論が発生する。

  • コードが歪む

    • これはレビュアーに修正依頼の意図がなく、純粋な質問をした場合に発生しうる。真意を探りすぎたレビュアーが既存のコードを過剰に調整してしまい、結果としてコードが歪んでしまう場合がある。

質問形式自体にこういう「こじれ」のリスクが有ることを意識したほうが良い。

なぜレビュアーは質問形式を使ってしまうのか

以下のような理由がある。質問形式のレビューは楽なのだ。

  • 指示することに対するためらいがある

  • 具体的な修正案がわからない、自分が想定する修正が最適なのか自信がない

  • レビュイーが自発的に気付き、修正することに対する教育的な期待を持っている

上二つの理由は心理的だったり技術的な問題であるため説明は省略するが、最後の理由は注意したい。確かにレビューのコメントでレビュイーが自身のコードの問題点に気付き、それの修正を行うことはあるが、それによる教育的なメリットはどれぐらいあるのだろうか。おそらくほぼ無いと言っていいだろう。仮にあったとしても、それは質問形式を行ったことによるものではない気がする。

より建設的なレビューをするために

  • 提案をする(コスト:低)

    • 提案形式で済むのなら、積極的に提案形式でレビューしたほうが良い。先に述べた通り質問形式は「こじれ」を引き起こすリスクを持っている

    • 答えを与えることに対する教育的なデメリットを感じる人もいるが、それは無いし、自発的な修正による教育的なメリットもない。もしそれが期待できるような状況ならば、「勉強も兼ねて周辺を調査し、最適だと思う手法を提案してください」ぐらいまで伝えた方が良い。

  • 純粋な質問であることを伝える(コスト:低)

    • 疑問に思ったときに使う。「ask」や「純粋な質問なのですが、」をつけるだけでいい。レビュアーが余計な真意を探る「こじれ」を減らせる。

  • 具体的な指摘をする(コスト:中)

    • 何が問題なのかを伝え、それと期待する修正内容を伝えるとより良い。

  • 指摘内容の意図や背景を伝える(コスト:高)

    • 技術的な差やドメインの差がある場合に必要になる。

コストの高いレビューをレビュアーに期待するのはやめる

自分がレビューするときはこれらのことを意識していきたいが、一方で人のレビューにこれらを期待するのは少し虫が良すぎるかもしれない。なぜレビュアーが質問形式を使うのかと言うと、それが楽だからだ。

雑なレビューを受け取ると少しがっかりすることもあるが、まずはレビューのために時間を割いてくれた相手に感謝の気持ちを持つようにしよう。