集団でソフトウェア開発をするときは、コードレビューをしたりされたりしながら進むのが当たり前となった昨今。
僕が自然と心掛けるようになったコードレビューお作法などをお気楽に書いてみます。
安易に「良い」とか「悪い」とか、一次元的な評価を下すのはよす
他人のコードに対し、絶対的に「良い」とも絶対的に「悪い」とも、「大正義」とも言わないようにしている。
コードの書き方や設計の出来栄えは、評価するための色々な物差しがあって、「良い」「悪い」の2つに1つ、斬って捨てられるようなものではないと思う。(むしろ、そういう単純なものではないからこそおもしろいんじゃないだろうか……?)
どのような指標をもちだすか、どういった利点/欠点に注目するのか、どんな畑で育ってきたプロダクトか、状況によって下すべき評価は変わるし、数えきれないほどある 設計パターンや慣習、従うべき原則、美的感覚のなかには、相反する「正しさ」が同時にたくさん存在してる。
- OOP的な設計に強い人が、拡張性の必要な部分のみに最小の継承ツリーをつくり、うまく設計できたと考えているが、実は、関数型パラダイムを心得ている人にとってはその継承すら回避すべきものとして見ている。
- 自然言語に近い形で読み下せることに重きを置く人が、メソッドチェインでかっこよく書けるようなI/Fをつくり、可読性が上がったと考えているが、実は、オブジェクト間の依存関係を最小に保つことに関心がある人の目には、Fatで醜いオブジェクトに映っている。
- リアクティブプログラミングが上手な人が、状態管理を廃して宣言的な記述をやりきったが、タイミング不定の入力の数がどんどん増えた場合に、どう考えても柔軟性がないことがあらわになる。
- CPUの気持ちになって型のサイズを減らしているつもりだったが、実はGPUの気持ちになると4要素が一体になっているほうが1要素x2より無駄がなかったり。
- デメテルの最小知識の法則とかを覚えた人が、用途に応じて専用の型をつくり、結合度を弱めようと努力しているが、パフォーマンス面では明らかに不利になっている。
- 英単語は省略せずに使うほうが可読性が高いと考える人が、長めのわかりやすい変数名をつけるよう心掛けているが、実は、数学や関数型に馴染みのある人にとっては、自明な式では 一文字変数も併用したほうが認知負荷が小さくよっぽど読みやすいと考えている。
- DRY原則が正義だと思っている人が、コードを共通化してリファクタしたと思っているが、実は、そこはたまたま重複していただけで、意味的には根本的に異なっているために、抽象度が上がっておらず、柔軟性を損ねている。
- 1つのメソッドが長いことは悪だと考えている人が、メソッドを分割してリファクタしたと思っているが、実は、割った処理が部分的に複数回呼ばれていないことが保証されなくなったので、安全面は劣化している。
- コンストラクタで初期化を全ておこなえば絶対に初期化忘れがないという考えを推していたが、実は、例外安全性を考慮してきた経験をもつ人の観点でみれば、副作用のあるコンストラクタは許しがたい。
- etc… etc…
結局、どういった面で「良い」のか、どういった指標において「悪い」のか、意見の一致をみないままに、「だめ」とか「神の存在を感じた」などと言ってみたところで、意見が割れた場合に退屈なことになる。
正しさには表と裏というものがあって、反対側から見てる人に対して一方的に「これは良くないな」とか言っても、意味がない。
あのとき、「このコードは綺麗に書けたなあ」なんて思っていた自分。時間が経って、信じているパラダイムが変わってしまえば、一瞬にして色褪せてしまう。綺麗かどうかなんて、そんな儚いものを、さも相手も自分と同じバックグラウンドや観念を持って当然とばかりに放り投げるのは、失礼にあたる、と、思うようになった。
もちろん、べつに理屈をこねなくても、直感的に「こりゃだめだな」と目を覆いたくなったり、「よくできている」と、関心したりはしている。それはもう、一目みた時点で、感じとっている。人間のする仕事だから、ある程度は直感を最初の道標にするのも当前だ。
しかし、自分の直感に過ぎない「なんかダメ」みたいな気持ちを、未加工のままパスして、相手にその正体の根拠を推察してもらい、吟味してもらい、判断してもらうのは、お互いに時間の浪費だとおもう。どんな価値観に基いてどんな面が「悪い」のか、具体的に説明するように気をつけている。
例)
- × ここなんか汚ねえ気がする
- ◯ 変数を変更できるスコープが非常に広いため、意図しない書きかえのリスクが高そうです。この場合は xxxをつかうと動作がより自明になるし、短く書けます。実装はネイティブなのでパフォ面でも有利でっす
もちろん、つーかーで通じる相手に対しては、長々と説明する必要なんてない。しかし、一度目はすくなくとも説明する努力は必要だろう。こちらの意図がつーかーで伝わるようになるその日が来るまでがんばろう。
謝罪する必要のないのに安易に「すみません」とか書かない
謝る必要のないところで「すみません」とか「:dogeza:」とか書かないようにしている。
直して欲しい箇所を伝える、相手の間違いを指摘する、そうした行為は立派に責任を果たしているのだから、謝るようなことじゃない。
謝る、と言うということは、何か自分に否があると表明することであって、そもそも、改めるつもりなんてなく、まったく同じ行為を次回もまた進んでやるだろう場合は、謝ってはいけない。いったいなにを謝っているのか、よくわからないし、簡単にでてくるすみませんには20円くらいの価値しかない。
相手が強気だからという理由だけで、謝って譲ってしまうと、問題を先送りすることになる。一度OKを出したのに二度目はNG、みたいなレビューが発生する。けっきょくどうしたいのか、わけがわからない。人間関係の問題とコードの問題は分離しよう。
僕は、油断すると口先だけでのすみませんを、言ってしまい、すみませんに40円くらいの価値しかない人間なので、意識的に気をつけている。
それに、謝りながら正しいことを言われても、それが正しいのかなんなのか、意図するところが曖昧に感じないだろうか? 無駄な訂正や謝罪は、ただでさえノイズの多いテキストのコミュニケーションにおけるノイズをさらに増やす。
いざというとき、すみませんに1万5千円くらいの価値がある人間でいたいもの……。
例)
- × 大変申し訳ありませんが、テストがコケているようです。
- ◯ テストがコケているみたいです。
そのかわり、間違えはきちんと訂正する
間違えたことを言ってしまったら、しっかりと訂正する。うやむやにしない。これはとても大事だと思う。
間違えたことを間違えた、と言える、わからないことをわからないと言える、そういう状況下でなければ、本心で考えている問題意識や意見・疑問が表にでてこない。
間違えたことを言ってはいけない、という縛りがあると、人は主観的な意見を一切言わなくなる。「正しい」とお墨つきがある情報や権威がある情報に頼る。だけど、そんな借りものの意見では議論にならない。
(たまに、かたくなに考えを変えない人というのがいるけど、その考えというやつが誰かの受け売りで、自分の意見ではないために、柔軟に変えたくても変えようがない、という場合がけっこうあります)
マイクロサービスがいかにメリットのあるアーキテクチャか、という主張が書かれているblogのURLを貼る、みたいな書き込みよりも、「ぼく、マイクロサービスがどうしてメリットが上回るのか未だに理解できていないです 🤔」みたいな当事者としての主観的な見方を書いてもらったほうが、結局は有意義だ。
また、間違えを自覚しているのに、それを表明しないまま話をすすめると、これまたノイズだらけになって、どこが結論なのか不明瞭になり、次のステップに進みずらい。
- × まあ、そういう場合もあるんだけど。そうじゃない場合もあるから、こう書いてたんだよね。
- ◯たしかに そういう場合がありえました。絶対ない、と書いたのは僕の勘違いでした。
どうしてほしいのかはっきり表明する
自分の意見を闇雲に書くだけではなく、結果として相手にどんな行動を期待するのか、わかるようにしておく。
これがないと、議論が宙を浮いたまま結論に収束しずらい。
- 直してもらわないとマージできない
- 直しても直さなくてもどっちでも良い。ささいな改善点を共有したかっただけ
この2つのどちらかなのかは常に表明できていると良いとおもう。
指摘される側になったら、
- 直すつもりです。
- 理解できなかったので、教えてほしい。
- 直す必要がないと思ってます。
この3つのうちどれなのかが表明できていると良いと思う。
直すつもりがない場合には特にはっきり言う。なぜ直さないのかをきちんと説明する。立場を表明した上で、直すのか直さないか協議にもっていって結論をだす。もちろん、直さない場合も「すみません」は必要ない。指摘してくれたことに「ありがとう」と言おう。そして、同意したなら、二度おなじレビューはもらわないようにする。その 積み重ねで効率が良くなっていくので、直す必要がないと思っているのに、嫌々なおしたりするのは長期的みて時間の無駄。
そんなかんじです
これはあくまで僕のこれまでの失敗に基いて、気をつけていることです。ルール化したり誰かに強制したりしたことはとくにありません。
やっぱり、お互いのことをよく知り尽くしている者同士がなにかと楽だとおもいます。こうした作法を色々省略しても万事うまくいきます。
