「戻り値の型も同じだし、この処理は一つの共通メソッドにまとめよう」
そうやって「いいことをしたつもり」で共通化を進めた結果、
- 1メソッドに複数の振る舞いが混ざる
- 分岐だらけ・フラグだらけになる
- 仕様が追えない
- テストケースが爆発する
…という状態になり、「あれ、これ本当に DRY (Don’t Repeat Yourself) なのか…?」と自分で首をかしげる、という経験はないでしょうか。
この記事は、「やりすぎた共通化」からあえて戻りつつ、テストしやすい設計に組み直す話です。
サンプルコードは Java を前提にしていますが、考え方自体は他言語にも応用できると思います。
想定読者
- 「共通化したはずなのに保守がつらい」と感じている人
- 既存の共通メソッドがフラグだらけで触るのが怖くなってきた人
- DRY を守りたいけれど、「やりすぎ」のライン感覚を掴みたい人
1. なぜ「やりすぎた共通化」がつらくなるのか
自分がハマったパターンをざっくり書くと、こんな感じです。
- 「戻り値の型が同じだから」という理由だけで、別々のユースケースを 1 つのメソッドに押し込める
- メソッド内で
modeやtype、isUpdateなどのフラグ引数・区分値で分岐する - ユースケースが増えるたびに
if/switchが増殖する - 呼び出し元によって「ほぼ決まったパス」しか通らないのに、メソッド内では全部ごちゃ混ぜ
その結果、こうなります。
- 「このメソッド、結局何をしてるの?」を一文で説明できない
- 仕様変更の影響範囲が直感的に読めない
- テストケースを洗い出すと、組み合わせで爆発する
つまり、DRY を守ったつもりで、変更容易性とテスト性を自分で悪化させている状態です。
ここで痛感したのが、
共通化の単位は、「型」や「一部のロジック」ではなく、 「振る舞い」 で見るべき
ということでした。
2. 共通化の単位を「型」から「振る舞い」へ
DRY (Don’t Repeat Yourself) はよく「重複コードをなくせ」と誤解されますが、本来は
同じ知識(ビジネスルール)の重複をなくす
という考え方です。
『The Pragmatic Programmer』(David Thomas, Andrew Hunt)の表現を借りると、「同じ知識は、単一で、明確で、信頼できる表現を持つべき」というイメージです。ここでいう「知識」がビジネスルールであり、それをどこにどう表現するかが DRY の本質になります。
- 戻り値の型が同じ
- 同じデータ転送オブジェクト(DTO: Data Transfer Object)を返している
といった表面的な共通点は、「ビジネス的な意味で同じ振る舞い」かどうかとは別の話です。
たとえば、こんなケースを考えます。
- 売上登録用の検索
- 請求書発行用の検索
が、たまたま今は同じ DTO を返しているとします。
でも、将来を考えると、
- 追加される検索条件
- 必要とされる項目
- バリデーションルール
は、それぞれのユースケースごとに違う方向へ進化していくかもしれません。
そのときに、
「型が同じだから」という理由だけで 1 メソッドに押し込んでしまう
と、のちのち変更がつらくなります。
そこで視点を変えて、
- 「将来の変わり方」が違いそうなものはメソッドから分ける
- 「本当に同じ知識・ルール」だけを、別メソッドとして共通化する
という方針に切り替えます。
共通化の基準を「型」ではなく「振る舞い(ドメイン上の意味)」に置き直すことが、
やりすぎ共通化から戻る最初の一歩になります。
3. 基本の武器:関数の抽出(Extract Method)
いきなり高度な設計パターンを持ち出さなくても、まずは 関数の抽出(Extract Method) だけでかなり戦えます。
3.1. 典型的な「全部入りメソッド」の例
まずは、ユーザ登録処理が 1 メソッドに全部入りしてしまっている例です。
public class UserRegistrationService {
private final UserRepository userRepository;
private final Mailer mailer;
private final Logger logger;
public UserRegistrationService(UserRepository userRepository, Mailer mailer, Logger logger) {
this.userRepository = userRepository;
this.mailer = mailer;
this.logger = logger;
}
public void register(UserForm form) {
// 入力チェック
List<String> errors = new ArrayList<>();
if (form.getName() == null || form.getName().isBlank()) {
errors.add("名前は必須です");
}
if (form.getEmail() == null || !form.getEmail().contains("@")) {
errors.add("メールアドレスの形式が不正です");
}
if (!errors.isEmpty()) {
throw new ValidationException(errors);
}
// 既存ユーザ重複チェック
if (userRepository.existsByEmail(form.getEmail())) {
throw new DuplicateUserException("既に登録済みのメールアドレスです");
}
// ユーザエンティティ生成・保存
User user = new User(
form.getName(),
form.getEmail(),
LocalDateTime.now()
);
userRepository.save(user);
// ウェルカムメール送信
String subject = "ご登録ありがとうございます";
String body = String.format("%s さん、ようこそ!", user.getName());
mailer.send(user.getEmail(), subject, body);
// ログ出力
logger.info("User registered: email=" + user.getEmail());
}
}
やっていること自体はそこまで難しくないのですが、
- 入力バリデーション
- 重複チェック
- エンティティ生成と保存
- メール送信
- ログ出力
といった、性質も変更頻度も違う処理が 1 メソッドに詰め込まれています。
この状態だと、
- 「バリデーションルールだけ変更したい」
- 「メール本文だけ A/B テストしたい」
- 「ログ出力の粒度だけ変えたい」
といったときに、テスト観点と変更範囲が全部くっついてしまいます。
3.2. やること単位で素直に分ける
これを、「やっていること」単位で素直に分割してみます。
public class UserRegistrationService {
private final UserRepository userRepository;
private final Mailer mailer;
private final Logger logger;
public UserRegistrationService(UserRepository userRepository, Mailer mailer, Logger logger) {
this.userRepository = userRepository;
this.mailer = mailer;
this.logger = logger;
}
public void register(UserForm form) {
validate(form);
checkDuplicate(form);
User user = createUser(form);
userRepository.save(user);
sendWelcomeMail(user);
logRegistration(user);
}
private void validate(UserForm form) {
List<String> errors = new ArrayList<>();
if (form.getName() == null || form.getName().isBlank()) {
errors.add("名前は必須です");
}
if (form.getEmail() == null || !form.getEmail().contains("@")) {
errors.add("メールアドレスの形式が不正です");
}
if (!errors.isEmpty()) {
throw new ValidationException(errors);
}
}
private void checkDuplicate(UserForm form) {
if (userRepository.existsByEmail(form.getEmail())) {
throw new DuplicateUserException("既に登録済みのメールアドレスです");
}
}
private User createUser(UserForm form) {
return new User(
form.getName(),
form.getEmail(),
LocalDateTime.now()
);
}
private void sendWelcomeMail(User user) {
String subject = "ご登録ありがとうございます";
String body = String.format("%s さん、ようこそ!", user.getName());
mailer.send(user.getEmail(), subject, body);
}
private void logRegistration(User user) {
logger.info("User registered: email=" + user.getEmail());
}
}
こうしておくと、
-
registerは「ユーザ登録の手続きの流れ」を示すだけ - 実際のロジックは、目的別の小さなメソッドに閉じ込められる
という形になります。
その結果、
- バリデーションだけを変えたい →
validateだけ見ればよい - 重複チェックの仕様を変えたい →
checkDuplicateだけ見ればよい - メール本文の変更やテスト →
sendWelcomeMailに集中できる
といった形で、変更したい箇所と読むべきコードの範囲が素直に対応しやすくなります。
また、単体テストを書くときも、
-
validateのテスト -
checkDuplicateのテスト -
createUserのテスト
のように、狙った振る舞いごとのピンポイントなテストを書きやすくなります。
4. 「やりすぎた共通化」から戻る実戦パターン
ここからは、この関数の抽出を応用した、より実践的なパターンを紹介します
パターン1:フラグ引数を用途ごとのメソッドに分割する
まずは典型的な「フラグ引数」パターンです。
public Result process(Order order, boolean isUpdate) {
// 共通の前処理
validate(order);
if (isUpdate) {
// 更新専用の処理
update(order);
} else {
// 新規登録専用の処理
create(order);
}
// 共通の後処理
notify(order);
return Result.success();
}
一見きれいに共通化されているように見えますが、
- 新規と更新でテスト観点が違う
- 今後、更新にだけ処理を足したい・削りたい
といったときに、ひとつのメソッドに両方の責務が載っていることが足かせになります。
これを、用途ごとに分割します。
public Result createOrder(Order order) {
validate(order);
create(order);
notify(order);
return Result.success();
}
public Result updateOrder(Order order) {
validate(order);
update(order);
notify(order);
return Result.success();
}
コード量としては少し増えますが、
- メソッド名だけで「何の処理なのか」が分かる
- テストも「create 用」「update 用」で分けやすい
- 共通なのは
validate/notifyだけ、という構造がはっきり見える
というメリットがあります。
フラグ引数が出てきたら、メソッド分割を検討する
くらいの感覚で見ると、やりすぎた共通化を早めに防ぎやすくなります。
パターン2:モード/区分ごとの分岐を、メソッドレベルで分ける
次にありがちなのが、mode や type で分岐しまくるパターンです。
public void execute(Mode mode, Context ctx) {
switch (mode) {
case REGISTER:
// 登録用の処理
...
break;
case UPDATE:
// 更新用の処理
...
break;
case CANCEL:
// 取消用の処理
...
break;
}
}
モードが増えるたびに switch は太り、
仕様変更のたびにこの巨大メソッドを開くことになります。
ここで一旦、「モードごとにメソッドを分ける」方向に戻します。
public void executeRegister(Context ctx) {
// 登録用の処理
...
}
public void executeUpdate(Context ctx) {
// 更新用の処理
...
}
public void executeCancel(Context ctx) {
// 取消用の処理
...
}
呼び出し側が mode を持っているなら、そこで分岐するだけとなります。
switch (mode) {
case REGISTER -> service.executeRegister(ctx);
case UPDATE -> service.executeUpdate(ctx);
case CANCEL -> service.executeCancel(ctx);
}
これで、
- モードごとの仕様を追うときは、関係するメソッドだけ見れば済む
- テストは「モードごとに対象メソッドを分けて書く」形にしやすい
- あるモード専用で処理追加・削除したいときにも影響範囲が分かりやすい
ようになります。
「switch をなくしたいから 1 メソッドに押し込む」のではなく、
「モードごとにメソッドを分けた上で、呼び出し側で責任を持って切り替える」 ほうが、
結果としてスッキリする場面はかなり多いです。
パターン3:あえて一度「コピペ」してから、改めて共通化を設計する
すでに巨大な共通メソッドがあって、そこに新しいユースケースを追加しないといけない
という状況もよくあります。
そのときに、いきなり既存メソッドの中に if / switch を足すと、
「さらにカオスになる」 ことが目に見えています。
そんなときに有効なのが、
- 既存メソッドを丸ごとコピーして、新ユースケース専用のメソッドを作る
- 新ユースケース専用メソッドの中で、安心してロジックを変える
- 旧ユースケースと新ユースケースの「本当に共通な部分」が見えてきたら、そこだけを改めて共通化する
というステップです。
「コピペは悪」と思い込みがちですが、リファクタリングの途中ではむしろ、
- 既存ユースケースを壊さないための安全装置
- 振る舞いの違いを浮かび上がらせるための一時的な手段
として有効に使えます。
ポイントは、「コピペしたらそこで終わり」ではなく、
落ち着いてから共通化の単位を再設計するための足場として使うことです。
5. 共通化したい気持ちとの付き合い方
コードを書いていると、
「ここ、まとめたら気持ちよさそうだな…」
という誘惑に駆られます。
それ自体は悪ではないものの、やりすぎを防ぐために、自分の中にいくつか基準を持っておくことが大切です。
-
将来の変わり方を 少しだけ妄想してから共通化する
変わり方が違いそうなら、メソッドを分けておく -
テストを書いたあとに共通化する
テストが複雑になりそうであれば、メソッドを分けておく -
「一文で説明できるメソッド」だけ共通化する
メソッドの説明に2,3文必要になる場合は、メソッドを分けておく
6. まとめ
- DRY は本来「同じ知識(ビジネスルール)の重複をなくす」考え方
- 「戻り値の型が同じだから」という理由だけで共通化すると、将来の変更がつらくなる
- 共通化の単位は「型」ではなく「振る舞い(ドメイン上の意味)」で考える
- まずは関数の抽出(Extract Method)で、やること単位にメソッドを分ける
- フラグ引数やモード分岐は、用途ごとのメソッドに分割してスッキリさせる
- 一度あえてコピペしてから、落ち着いて共通化を再設計するのも有効
手元のプロジェクトで、
- フラグ引数だらけ
- 巨大な共通メソッドにモード分岐が盛りだくさん
といったコードを見つけたら、
「振る舞いごとに素直に分けてみる」 ところから、少しずつ戻してみるのがおすすめです。