レビューで褒められたコードほど、半年後に厄介になる──という逆転現象を見たことはないだろうか。
よくある筋書きはこうだ。似たような処理を書く2つの関数があり、レビュアーが「これ、ほぼ同じ処理だから共通化できますよね」と指摘する。書いた人も「確かに重複している」と納得し、1つの関数にまとめる。DRY(Don't Repeat Yourself=同じ知識・同じロジックを複数箇所に書かない、という原則)に従った、お手本のような修正だ。レビューは一発で通り、プルリクエストには拍手のスタンプが並ぶ。
そして半年後、その共通化された関数は、チームで「触るのが一番怖い場所」になっている。何が起きたのか。
典型的な崩壊パターン
次のような擬似コードを考える。注文の確認メールを送る処理と、注文の催促メールを送る処理が、最初はほぼ同じ内容だったとする。
function sendOrderConfirmationEmail(order) {
const body = buildOrderBody(order);
send(order.userEmail, "ご注文ありがとうございます", body);
}
function sendOrderReminderEmail(order) {
const body = buildOrderBody(order);
send(order.userEmail, "お支払いのお願い", body);
}
「本文の組み立てと送信処理が丸ごと重複している」と気づいた誰かが、1つの関数にまとめる。
function sendOrderEmail(order, type) {
const body = buildOrderBody(order);
const subject = type === "confirmation"
? "ご注文ありがとうございます"
: "お支払いのお願い";
send(order.userEmail, subject, body);
}
ここまでは、まだ害がない。だが実際の開発では、ここから要件が片方だけ変わっていく。
「確認メールは、法人向け注文だけ請求書PDFを添付してほしい」という要望が来る。担当者はsendOrderEmailに手を入れる。
function sendOrderEmail(order, type) {
const body = buildOrderBody(order);
const subject = type === "confirmation"
? "ご注文ありがとうございます"
: "お支払いのお願い";
let attachment = null;
if (type === "confirmation" && order.isCorporate) {
attachment = buildInvoicePdf(order);
}
send(order.userEmail, subject, body, attachment);
}
次に「催促メールは、3回目以降は上長にもCCを入れてほしい」という要望が来る。同じ関数にまた分岐が増える。
function sendOrderEmail(order, type) {
const body = buildOrderBody(order);
const subject = type === "confirmation"
? "ご注文ありがとうございます"
: "お支払いのお願い";
let attachment = null;
if (type === "confirmation" && order.isCorporate) {
attachment = buildInvoicePdf(order);
}
let cc = [];
if (type === "reminder" && order.reminderCount >= 3) {
cc = [order.managerEmail];
}
send(order.userEmail, subject, body, attachment, cc);
}
数か月後、この関数はtypeという名前の引数1つで2つの別物を分岐させる、読むのに頭を使う塊になっている。確認メールの担当者は催促メール側のif文も読まないと安心して直せない。テストも両方の分岐を踏まないと通せない。「重複をなくした」はずなのに、変更のたびに関係のない方の分岐まで目を通す必要が生まれ、変更コストはむしろ上がっている。
なぜこうなるのか──「似ている」と「同じ理由で変わる」は別の軸
ここでの誤りは、「コードの見た目が似ている」ことと「将来も同じ理由で変わる」ことを同じものだと扱ったことにある。確認メールと催促メールは、書いた瞬間のコードはほぼ同じでも、変わる理由(誰の要望で、いつ、何のために変更が入るか)はまったく別だった。確認メール側の変更は「請求書対応」という営業要件、催促メール側の変更は「督促フローの強化」という別の要件から来ている。
ソフトウェア設計で言う凝集度(cohesion=モジュールの中身がどれだけ目的でまとまっているか)と結合度(coupling=モジュール同士がどれだけ依存し合っているか)の言葉で言うと、このケースでは「見た目の重複を消す」ことを優先した結果、本来独立しているべき2つの関心事が1つの関数に押し込められ、結合度が上がってしまった。片方を直すには、もう片方の分岐を読んで壊さないことを確認しなければならない。変更が本来の範囲を越えて波及する状態、これが「間違った抽象化」の正体だ。
この問題は実務でよく知られていて、Rubyコミュニティで知られるソフトウェア設計者のSandi Metzは、「重複は、間違った抽象化よりもずっと安い」という趣旨の主張を広めたことで知られている。重複したコードはその場でコピー&ペーストされた2つの独立した塊でしかなく、片方を直しても他方に影響しない。一方、間違った抽象は共有された1つの実体であり、そこに手を入れると関係のない利用箇所まで揺れる。見た目のコード行数は減っても、変更の見積もりとレビューの負荷は増える。
DRY原則そのものは、重複を無条件に悪とは言っていない。本来のDRYは「同じ知識・同じ業務ルールが複数箇所に存在し、片方だけ直して矛盾が生まれる」ことを避けるための原則で、「見た目が似ているコード」をすべて1箇所にまとめろ、という話ではない。それでも現場では、コードレビューで「ここ重複してますね」のひとことが独立した議論抜きに即・共通化のゴーサインとして使われがちで、これが「DRYの教条化」として度々議論の的になっている。どちらの立場にも理があり、「重複は常に悪」と「抽象化は常に善」のどちらも単純化しすぎている、という点では議論の多くが一致している。
共通化すべきタイミングの見分け方
実務で効く判断軸は、「見た目が似ているか」ではなく「変わる理由が同じかどうか」で考えることだ。具体的には、次の問いを共通化の前に自分に向ける。
- この2つのコードが将来変わるとしたら、同じ要望・同じ担当・同じ業務上の理由で変わる可能性が高いか。理由がばらばらなら、今は重複を許して構わない。
- 共通化した後、呼び出し側ごとに分岐するフラグ引数(
typeやisAdminのような引数)が必要になりそうか。必要になるなら、それは2つの別物を1つの型に押し込める前兆であり、共通化よりも「共通部分だけを小さな関数に切り出し、呼び出し側は別々に保つ」方が安全なことが多い。 - 重複が3回目に現れたかどうかを見る。経験則として、2回の重複ではまだパターンかどうか判断できない。3回目に同じ形の重複が出てきて、かつ変わる理由も同じだと確認できたときに共通化する、という「Rule of Three」と呼ばれる考え方は、早すぎる抽象化を避けるための目安としてよく引かれる。
だからこそ、一度共通化した関数を後から「また分割に戻す」判断は、設計の後退ではない。むしろ、変わる理由が違う2つのものを無理に1つにまとめていたという誤りに気づき、最も健全な形に戻す判断であることが多い。「分割する=元に戻す=失敗」と捉えてしまうと、その関数は誰も手を出せないまま肥大化し続ける。
共通化そのものが悪いわけではない。危ういのは、「重複を消すこと」自体をきれいさの指標にして、変更理由の違いを確認せずに済ませてしまうことだ。コードの目的は見た目を整えることではなく、半年後に誰かが安全に、狭い範囲で変更できる状態を保つことにある。レビューで「重複してますね」と言われたら、一度「これは同じ理由で変わる重複か」と問い返してみる。
設計やリファクタリングの考え方を扱った定番書は何かを、実際に開発者が薦めている書籍のデータで確かめられる場所を作っている。気になる人は見てみてほしい。
https://s-tasklog.com/books/general