こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。
この記事は、毎週木曜の定期連載 "Weekly Report" 共有の第 51 回です。 LINEヤフー社内には、高い開発生産性を維持するための Review Committee という活動があります。ここで集まった知見を、Weekly Report と称して毎週社内に共有しており、その一部を本ブログ上でも公開しています。（Weekly Report の詳細については、過去の記事一覧を参照してください）
確信的な質問
ある開発者が、以下の関数 getOnlineUserStates のコードレビューの依頼を受けた状況を想定し ましょう。
fun getOnlineUserStates(userIds: Set<UserId>): Map<UserId, UserState> {
if (userIds.isEmpty()) {
return emptyMap()
}
return userIds.asSequence()
.map { ... /* Convert to Pair<UserId, UserState> */ }
.filter { ... /* Filter only online users */ }
.toMap()
}この関数は、オンラインであるユーザの状態を Map として返す関数です。最初の if では、userIds が空の場合に早期リターンしています。
しかし、asSequence() 以降の処理は userIds が空でも正しく動くのに加え、この早期リターンによる性能向上はかなり限定的です。そこで、レビューアはこの早期リターンを削除し、コードの単純さを優先したほうが良いと考え、以下のようなレビューコメントを送りました。
Reviewer
この早期リターンの目的は何ですか？
このコメントに対し、レビューを依頼した開発者 (オーサー) は以下のように返答しました。
Author
userIdsが空のときは、asSequence()以降の処理は無駄になるため、早期リターンを適用しています。
以降のレビューアとオーサーのコメントのやり取りを以下に示します。
Reviewer
しかし、
userIdsが空になる可能性はそれほど高くなく、かつ、空のuserIdsに対するasSequence()以降の計算時間も十分短いです。それでも、この早期リターンは必要ですか？
Author
はい、効果が小さいとはいえ、確実に性能は良くなるはずなので、この早期リターンは必要です。
このやり取りではレビューアの意図がオーサーにうまく伝わっていないように見受けられます。どこに問題があるでしょうか？
何が言いたいかわかる？
このミスコミュニケーションの原因の 1 つに、早期リターンの削除はレビューアの「要求」であるのにもかかわらず、単なる「質問」にも読み取れてしまう点が挙げられます。オーサーの視点で見ると、質問に答えているのにもかかわらず、議論の焦点が明確になっていないように感じられるでしょう。通常の文章や会話では、疑問文は婉曲や丁寧な表現をする上で重要な役割を果たします。しかし、 コードレビューにおいては要求と質問は明確に区別できるように書かれるべき です。
また、要求や提案を行う場合は、その利点も説明できるとなお良いです。ただし、どの程度の説明が必要になるかは、オーサーと共有できている背景や文脈にも依存します。
Reviewer
userIdsが空の場合でもasSequence()以降のロジックは正しく動くため、この早期リターンを削除してください。(userIdsが空の時は、早期リターンがある方がわずかに性能が良くなるのですが、その効果は限定的です。その場合は、性能よりもコードの単純さを優先したほうが利点が大きいと思います。参考: https://speakerdeck.com/munetoshi/code-readability-session-1-ver-2-ja?slide=44 )
もちろん、確信が持てない点や理解できていない点があるならば、質問することは適切です。不明な点を放置すると見落としが発生 しやすくなり、間違った推測によるミスコミュニケーションが発生しやすくなります。それに加え、不明な点の調査をオーサーに依頼することで、レビューアとオーサーの負荷のバランスを調整することもできます。
レビューコメントを何でも質問の形式にするのではなく、意図が伝わるような形式を選択することが必要です。そのためにも、レビューコメントを書く際は常に そのコメントが要求、提案、質問のどれに分類できるか を考えるとより良いです。
そのコメントは何？
そのレビューコメントが要求、提案、質問のどれかを明確にするために、特定のフレーズやタグを使うのも良い方法です。
- 要求: 「...してください」("Please do...")、「...する必要があります」("We have/need to do...")、[Request]、[Requirement]、[Must]
- 提案: 「...したほうが良いかもしれません」("It would be nice to do...")、「...する方法もあります」("An option is to do...")、[Suggestion]、[Optional]、[Nice to have]、[IMO]
- 質問: [Question], [Confirmation], [Just curious]
一言まとめ
レビューコメントを書く際は、それが要求、提案、質問のどれに相当するかを考える。
キーワード: code review, review comment, communication