地頭が良すぎる若手が、1000行のメソッドを書いてきた｜レビューを断った日のこと

※本ページはプロモーションが含まれています

コードレビューで、手が止まったことがあります。

画面をスクロールしても、スクロールしても、メソッドが終わらない。行数にしておよそ1000行。ファイル全体ではなく、ひとつのメソッドの話です。

そして、それを書いたのは、チームで一番地頭の良い若手でした。

スクロールしても終わらないメソッドが、レビューに出てきた

帳票を出力する処理でした。正確には、出力する前の集計をおこなう部分です。

条件分岐があり、ループがあり、例外処理がある。それ自体は珍しくありません。問題は、それらがすべてひとつのメソッドの中に、順番に積み上がっていたことでした。

読み進めると、途中で分からなくなります。今どの処理を読んでいるのか。この変数はどこで値が入ったのか。ページを戻って確認して、また読み進める。その繰り返しでした。

誤解のないように書いておくと、動作に問題はありませんでした。テストも通っていました。動くコードではあったのです。

動いているものを前にして、レビューを担当する立場としてどう言うか。私はしばらく考えました。

「レビューする前に、まず分割して」

結局、私が口にしたのはこの一言でした。

指摘ではありません。突き返しです。レビューを始める前の段階で、いったん止めました。

返ってきたのは「えっ」という一言でした。

彼女は自信を持ってレビューに臨んでいました。無理もありません。動くものを、期日までに、最後まで書ききったのですから。直すべき箇所を指摘されるつもりで来たら、読む前に返された。驚くのが当然です。

ここは、書き手の落ち度を責める場面ではありませんでした。彼女は手を抜いていない。むしろ逆で、最後まで書ききる集中力があった。

ただ、レビューが成立しなかったのです。読めないものは、品質を確認できません。

書いたのは、地頭が良すぎる若手だった

ここが、この話の一番おかしなところです。

彼女は入社2年目。頭の回転が速く、飲み込みも早い。仕事を任せたくなるタイプの若手でした。

1000行のメソッドを書けたのは、能力が低かったからではありません。1000行が、頭に入ってしまったからです。

最初から最後まで、処理の流れを記憶していられる。どこで何をしているか、常に把握できている。だから彼女の中では、そのコードは十分に見通せていました。分割する理由が、本人にはなかったのです。

地頭が良い。良すぎた。それが、分割の動機を奪っていました。

読みにくいコードを書くのは、能力が低い人だ。私たちはつい、そう思い込みます。でも実際には逆のことも起きます。自分の頭に収まってしまう人ほど、他人の頭に収まらないことに気づけません。

これは技術力の問題ではなく、視点の問題です。

動くコードと、人に渡せるコードは違う

彼女に伝えたのは、三つでした。

1. レビューしづらいコードは、それだけでだめなコード

読む側が内容を追えないなら、品質を確認する工程そのものが機能しません。レビューは形式ではなく、間違いを見つけるための仕組みです。読めない時点で、その仕組みが止まります。

2. 1年後には、自分のコードも他人のコードになる

今は全部覚えています。でも半年後、一年後の自分は他人です。別の案件を挟んだあとに戻ってきて、自分の書いたコードが読めなかった経験は、多くの人にあるはずです。

そのとき助けてもらえるかどうかは、今日の書き方で決まります。

3. 難しいことを簡単に書くほうが、ずっと難しい

難しいことを難しく書くのは、実は簡単です。複雑なものを複雑なまま出せばいいだけですから。

難しいことを、誰でも分かるように書く。そちらのほうがずっと難しい。整理する手間も、構造を考える時間も要ります。

三つ目を言ったとき、彼女の表情が変わったのを覚えています。

「動くものを作る」から一段上がった話をされている、と伝わったのだと思います。能力の高い人ほど、難易度の話には反応します。

彼女は納得して、分割して持ってきました。理由を説明したからです。「ルールだから」では、地頭の良い人は動きません。なぜそうするのかが腑に落ちれば、あとは早い人でした。

私も新卒のとき、やりすぎた

偉そうに書いていますが、私にも似た話があります。

新卒の頃、アセンブラのコードに「コメントを入れて」と指示されたことがありました。

私は1行ごとにコメントを入れました。全部の行にです。

返ってきたのは「さすがにやりすぎ・・・」という指摘でした。

言われた通りにやったつもりでした。コメントを入れろと言われたから、入れた。ただ、どこまで入れるかという加減を、私は知らなかったのです。

彼女と私は、方向が逆です。彼女は分割しなさすぎ、私は書きすぎ。

でも、形は同じでした。良かれと思ってやっただけ、加減を知らなかっただけ。

加減は、誰かに教わるまで分かりません。自分では気づけないから加減なのです。だから、諭す側が要ります。

私が新卒のときに指摘してもらえたように、今度は私が言う番だった。それだけの話です。彼女を叱る資格が私にあったわけではありません。順番が来ただけでした。

分割の最低限のルールは、二つだけ

では具体的にどう分割するか。彼女に示したのは、二つだけです。

1. 全体が見えるメソッドと、個別に必要なメソッドに分ける

上から読んで処理の流れが分かる親メソッドを、ひとつ置きます。具体的な処理は、そこから呼び出す形にする。

こうすると、読む人はまず流れだけを追えます。詳細が必要になったときだけ、該当のメソッドを開けばいい。全部を頭に入れなくても読めるようになります。

2. 重複したコードはまとめる

同じ処理が二箇所にあるなら、いつか片方を直して、もう片方を忘れる日が来ます。必ず来ます。

そしてその不具合は、直した直後には出ません。しばらく経ってから、忘れた頃に出てきます。

なぜ二つに絞るのか。五つ挙げても、誰も覚えないからです。現場で使われないルールは、無いのと同じです。

この二つを満たすだけで、レビューは成立するようになります。完璧な設計の話ではありません。人に渡せる状態にする、最低ラインの話です。

そもそも、コーディング規約があればこうはならない

ここまで書いてきて、前提の話をしていなかったことに気づきました。

コーディング規約がある現場なら、まずはそれに従ってください。この記事に書いた二つのルールより、その現場の規約が優先です。

規約には、命名の付け方、インデントの深さ、メソッドの分割方針、コメントの書き方まで決まっていることが多いものです。それがあれば、レビューで「読みにくい」と言われても、何をどう直せばいいかが明確になります。

今回の1000行のメソッドが生まれたのも、突き詰めればそのプロジェクトに規約がなかったからです。

規約がないと、何が起きるか

規約がない現場では、こういうことが起きます。

• 各自の「良いと思う書き方」がそのまま採用される。全員が善意で書いているのに、ファイルごとに流儀がばらばらになります
• 指摘が個人の好みに見える。「私はこう書くほうが読みやすいと思う」と言われたとき、規約がなければ決め手がありません。どちらも一理あるまま平行線になります
• レビューで揉める。本来は不具合を見つける場なのに、書き方の議論に時間を取られます
• 新しく入った人が困る。どれを手本にすればいいのか分からない。結局、隣の席の人のやり方を真似することになります

1000行のメソッドを突き返したとき、私が理由を三つも説明したのは、「規約にこう書いてあるから」と言えなかったからでもあります。拠り所がないので、一つずつ納得してもらうしかありませんでした。

規約が一行あれば、あの会話は一分で済んだはずです。

規約がない現場では、どうするか

いきなり分厚い規約を作ろうとすると、作ること自体が目的になって頓挫します。実際、そうなった現場を何度か見てきました。

おすすめは、揉めたところだけを一行ずつ足していくやり方です。レビューで議論になった項目を、その場で決めて書き留める。それだけで、同じ議論が二度と起きなくなります。

この記事に書いた二つのルール、つまり「全体が見えるメソッドと個別のメソッドに分ける」「重複はまとめる」も、そうやって足していく最初の一行として使えます。

拠り所が欲しいなら、この一冊

規約をゼロから考えるのが大変なら、共通の下敷きを一冊持っておくと早いです。

『リーダブルコード』は、名前の付け方から関数の分割まで、「なぜそう書くと読みやすいのか」を理由つきで説明している本です。今回の話でいえば、私が彼女に伝えた三つの理由が、もっと丁寧に書かれていると思ってください。

チームの何人かが同じ本を読んでいると、レビューが早くなります。「ここは説明用の変数を挟んだほうが読めるかも」といった言い方が、そのまま通じるからです。共通の言葉ができると、指摘が個人攻撃に聞こえなくなる。これが一番大きい効果かもしれません。

薄い本ではありませんが、通して読む必要はありません。気になった章だけ拾い読みして、良いと思った項目をチームの規約に足していく。そういう使い方が現実的です。

よくある質問

メソッドは何行までなら許容範囲ですか？

行数で線を引くより、「上から読んで処理の流れが分かるか」で判断することをおすすめします。30行でも分岐が入り組んでいれば読めませんし、80行でも素直に上から下へ流れていれば読めます。

行数の基準を決めると、基準を満たすためだけの不自然な分割が始まります。目的は行数を減らすことではなく、読める状態にすることです。

動いているコードを、わざわざ直す必要はありますか？

今後まったく手を入れないコードなら、急いで直す必要はありません。判断の分かれ目は「この先、誰かが改修するかどうか」です。

改修する予定があるなら、読めない状態のまま置いておくほど改修の難易度が上がります。書いた本人が異動や退職でいなくなった後は、なおさらです。

指摘したら相手が萎縮してしまいそうで、言い出せません

「だめ」だけを伝えると萎縮します。理由とセットで伝えると、受け取り方が変わります。

この記事の彼女も、「分割して」だけなら反発したかもしれません。1年後には自分も他人になること、レビューが成立しないこと、その理由を説明したから納得してくれました。相手が優秀であるほど、理由を求めます。

自分が書いたコードが読みやすいか、どう確かめればいいですか？

書いた直後は判断できません。自分の頭に内容が残っているうちは、どんなコードでも読めてしまうからです。

手軽なのは、数日空けてから自分で読み返すこと。それでも詰まるようなら、他人が読めばもっと詰まります。可能であれば、誰かに一度目を通してもらうのが確実です。

まとめ：分かるように書くのは、才能ではなく教わること

コードの品質を落とすのは、能力の低さだけではありません。

自分の頭がよく回るせいで、他人の頭を経由しないコードを書いてしまうことがあります。動いているし、テストも通る。だから問題は表に出てきません。

表に出るのは、書いた人がいなくなった後です。

動くコードを書けることと、人に渡せるコードを書けることは、別の能力です。そして後者は、たいてい誰かに一度言われないと身につきません。

もし今、あなたのレビューに読めないコードが出てきたら。書いた人を責める前に、その人が加減を教わったことがあるかどうかを思い出してみてください。

私も、教わった側でした。1行ごとにコメントを入れて、やりすぎだと言われた新卒が、いま こうして書いています。