今回やること
この記事では、共通化しすぎて読みにくくなった関数を、目的ごとに分け直します。
共通化は戻してもよいです。間違った抽象化を残すほうが危険です。
前提条件
- 関数とif文が読める
- DRY原則の基本を知っている
- リファクタリングは動作を変えずに構造を変える作業だと理解している
Step 1: 共通化しすぎたコードを見る
function validateText(value: string, type: "user" | "product") {
if (type === "user") {
return value.length >= 2 && value.length <= 20 && !value.includes(" ");
}
if (type === "product") {
return value.length >= 1 && value.length <= 80;
}
return false;
}
ユーザー名と商品名を1つの関数で扱っています。type によって処理が大きく変わっています。
Step 2: 問題を確認する
この関数の問題は、共通化したのに中で分岐していることです。
| 問題 | 内容 |
|---|---|
| 呼び出し側がtypeを覚える | "user" と "product" を間違えやすい |
| 仕様変更が怖い | 商品名変更がユーザー名処理へ影響する可能性 |
| 名前が曖昧 | validateText だけでは対象が分からない |
Step 3: 関数を分ける
function validateUserName(name: string) {
return name.length >= 2 && name.length <= 20 && !name.includes(" ");
}
function validateProductName(name: string) {
return name.length >= 1 && name.length <= 80;
}
コードは少し増えました。しかし、責務は明確になりました。
Step 4: 呼び出し側を読みやすくする
if (!validateUserName(input.name)) {
throw new Error("invalid user name");
}
呼び出し側に type を渡す必要がなくなりました。関数名だけで何を検証しているか分かります。
Step 5: 共通化を残す部分を探す
完全に共通化を捨てる必要はありません。
function isLengthBetween(value: string, min: number, max: number) {
return value.length >= min && value.length <= max;
}
長さチェックのように、本当に同じ知識なら小さく共通化できます。ただし、業務名を持つ関数は分けておきます。
よくあるエラー
| 状況 | 原因 | 対応 |
|---|---|---|
| 分けたらコードが増えた | 重複をゼロにしようとしている | 読みやすさを優先する |
| 共通関数を消すのが怖い | 共通化は常に正義と思っている | 変更理由で判断する |
| 分けた後に同じ修正が続く | 本当に同じ知識だった | 小さく再共通化する |
| テストが壊れた | 動作まで変えてしまった | 先にテストで現状を固定する |
まとめ
共通化しすぎた関数は、責務ごとに分け直して構いません。中で type や mode による大きな分岐が増えているなら、間違った抽象化のサインです。