TotT: レビュー依頼するコードはわかりやすさを
こんにちは、kubopです。
Googleにはトイレテスト(TotT)という文化があるようで、テストに関するTipsをトイレに貼り出し、テストに関する知識を全社で共有しているらしいです。
昨今はリモート勤務が広まり、TotTの実施は難しく、SlackやBotを用いてもなかなか浸透するかどうか…
そこで、noteに書きつつ自分が勉強するために、少しずつ読んで内容や、所感を書いてみようと思います。
※ 翻訳・解釈の間違いなどあるかもしれません。
その場合はこっそり教えてください。
Code Health: Understanding Code In Review
レビューのためにPRを送ってきた開発者は、自分よりも賢いと思いがちで、そのために彼らのコードを理解できない。
しかし、コードが理解しにくいのは、おそらく複雑すぎるのです。
もしあなたが使われているプログラミング言語に精通していれば、健全なコードを読むことは、あなたの母国語の本を読むのと同じくらい簡単なはずです。
ある開発者があなたにこのPythonのブロックを送ってきたとします。
def IsOkay(n):
f = False
for i in range(2, n):
if n % i == 0:
f = True
return not f
理解しようとするのに数秒以上かけてはいけません。
コードレビューのコメントで、"It's hard for me to understand this piece of code"(このコードの一部を理解するのは難しいです)、
あるいはもっと具体的に、"Please use more descriptive names here"(ここはもっとわかりやすい名前にしてください)と書き添えるだけでよいのです。
すると、開発者はコードを明確にして、再びレビュー依頼します。
def IsPrime(n):
for divisor in range(2, n / 2):
if n % divisor == 0:
return False
return True
開発者にコードの一部を明確にしてもらうだけで、根本的な改善につながることはよくあることです。
この場合、開発者はコードが読みやすくなったことで、パフォーマンスが向上する可能性に気づきました。
関数が素数でない場合にアーリーリターン出来、ループはnではなくn/2までしか回らなくなりました。
しかし、このコードを簡単に理解できるようになった今、多くの問題点が見えてきました。
例えば、0と1では奇妙な動きをしますし、他にも問題があります。
しかし、最も重要なことは、この関数全体を削除し、数が素数であるかどうかを検出する既存の関数に置き換えるべきであることが明らかになったことです。
コードを明確にすることは、開発者とレビュアーの両方を助けることになります。
要約すると、理解しにくいコードのレビューに時間を費やす必要はなく、コードを明確にするよう求めればよいのです。実際、このようなレビューコメントは、コード・レビュアーが持つ最も有用で重要なツールの1つです。
メソッドを分割してみるのも、一つの手段かもしれません。
🤔
レビュー依頼をしたコードがしばらくそのままになる、ということはよくあることですが、これはコード自体が複雑すぎるからというのがあるかもしれないなと思いました。
または、変更点や作成しようとする仕様が複雑であること。
これらは開発しているとつい自分の中では理解しているのでなかなか気が付きにくい。
そういった場合はレビュワーは「もう少しシンプルに」といった指摘をして理解しやすさを求めるのも一つの手かもしれません。
もちろん、HRTは忘れずに…。
Ruby意訳
def is_ok(n)
f = false
(2..n - 1).each do |i|
if n % i == 0
f = true
end
end
!f
end
def is_prime(n)
(2.. n / 2).each do |divisor|
if n % divisor == 0
return false
end
end
true
end
Licensed under a Creative Commons
Attribution–ShareAlike 4.0 License
この記事が気に入ったらサポートをしてみませんか?