はじめに
皆さん、コードレビューで「好みの問題ですが、〜」と言ったことや、言われたこと、ありませんか？
あるいは、言いはしなくとも
「（これは好みの問題だから指摘しなくてもいいか…）LGTM！」
と片付けてしまったことはありませんか？
「好み」と言われると、蕎麦とうどんどっちが好き？みたいな話に聞こえますが、
実際はそんな好き嫌いで片付けずとも、みんなちゃんとコードの書き方にロジカルな理由を持っているんじゃないか？
とたまに思うことがあります。
多分「好みの問題」と曖昧に片付けられてしまうケースの多くは、
コードを書いている本人としても、「なぜ自分はその書き方にしたいのか？」が明確になっていないのかもしれません。
「好む」ということは、きっと何かしらの好む理由（そちらのほうが優れたコードであると思う理由）があるはずで、
それを明確にしていけば、自分にとっても理解が深まるし、より良いコードレビューができるんじゃないか？
というのがこの記事で伝えたいことです。
例
新人エンジニアAさんが、以下の仕様を実装したときのコードレビューを例に考えてみます。
仕様書: 「掲示板を作る」
・ログインしないと投稿できないようにしたい
・成人向け掲示板も作りたい（未成年は投稿できない）
上記仕様を満たすために、「メッセージを投稿できるか？」を返却する canPostMessage というメソッドをつくりました。
class {
constructor () {
this.me = getLoggedinUser()
}
canPostMessage (bbs) {
if (!this.me) return false
if (this.me.age < 20 && bbs.forAdult) return false
return true
}
}
さっそくレビューに出してみると…、
先輩エンジニアBさん: 「こうすると1行ですっきり書けます！」
canPostMessage (bbs) {
return this.me && (this.me.age >= 20 || !bbs.forAdult)
}
新人エンジニアAさん: 「………。」
新人Aさんはなんとなく自分が書いたコードのほうが良いと思いましたが、あくまでなんとなくです。
これが例の「好みの問題」でしょうか？
新人Aさんは少しモヤモヤしつつも、
「大きな問題じゃないし、先輩に言われてしまったし、黙って提案を受け入れるか」と、
提案をソースコードに適用してしまいました。
（↑これがめっちゃ勿体無い！）
もっと掘り下げよう！
皆さんはどちらのコードが「好み」でしたか？
そして、それはなぜでしょうか？
この記事では、どちらのコードがより優れているかはそんなに重要ではありません。
重要なのは、新人Aさんが自分のコードの方が良いと思っていることと、
恐らく先輩Bさんにはその理由が伝わっていないということです。
「好みの問題」で片付けてしまった場合、このレビューはAさんの中にあるAさんのコードの良さが明らかにならないまま終わります。
新人Aさんがやるべきだったことは2つです。
- なぜ自分の書いたコードのほうが良いと思ったのか、自分の中で明確にする
- それをBさんに説明する
新人エンジニアAさんの考え
新人Aさんは、以下のようなイメージで、「文脈での分かりやすさ」を重視して書きました。
投稿できる？ (掲示板) {
もし (ログインしていないなら) 投稿できません。
もし (私が未成年なら、成人向け掲示板には) 投稿できません。
それ以外の場合、投稿できます。
}
Bさんのコードも同じく文にすると、以下のような感じです。
投稿できる？ (掲示板) {
ログインしていて、私が成人済みか、その掲示板が成人向け専用でないなら、投稿できます。
}
前者は、確かに3行使ってしまいますが、
仕様書をそのまま英語（プログラミング）にしたかのようで、意図を汲み取るのが容易です。
後者は、「ログインの有無」と「成人向け掲示板の制約」という異なる要素に対する条件が一文に混在してしまっているので、頭の中で分解しながら読み進めなければいけません。
「this.me.age >= 20 || !bbs.forAdult」
の部分も、頭の中で条件を逆転させて初めて「未成年は成人向け掲示板に書き込めない」という仕様を理解できます。
また、将来このメソッドに新しい条件を足すことになったときにも、前者のほうが綺麗に追加できそうです。
先輩Bさんに説明できたら？
もし先輩Bさんがこの説明を聞いて納得し、彼も新人Aさんのコードのほうが良いと感じたとすれば、
これは「好みの問題」ではなかったと言えないでしょうか？
そして先輩Bさんは新しい考えに出会うことができたし、新人Aさん自身も「なんとなく」だった自分のコードのメリットを明確にすることができました。
もし説明を聞いて意図を理解したうえでも、やはり先輩Bさんはすっきり1行で書くほうに魅力を感じるのであれば、
………それは「好みの問題」だと思います。
説立証ならず。
それでも、お互いが感じているメリットデメリットが曖昧なままレビューを終わらせるより、ずっと良いはずです。
おわりに
あらためて、僕が伝えたかったことは、
「なぜ自分はこの人の提案を採用したくないか」／「なぜ自分はこの人にこの提案をしたいのか」は、
まず自分の中で整理すれば、「好みの問題」で終わらせずにちゃんと説明できるケースが多いんじゃないか、
そうすると、コードレビューの質はより良くなるんじゃないか、ということでした。