同じ処理=共通化ではダメ？共通化の罠

はじめに

みなさんは共通化していますか？プログラムを書いていると、必ず意識するようになることだと思います。

自分も似たような処理を共通化してひとつにまとめたことがあります。しばらくはうまく回っていました。ただ、似ているけどちょっと挙動が違う処理が必要になるたびに、共通処理にif文の分岐が増えていきます。壊れてはいないけど、どんどん複雑になっていきました。あるとき、もうこれ以上分岐を増やせないと感じて、似たようなコードを持つ別のクラスを作ってしまいました。DRYにしたかったはずなのに、コードが重複しています。

「共通化は正義」だと思っていたのに、共通化したことで逆に身動きが取れなくなっていました。『オブジェクト指向設計実践ガイド』を読んで、この失敗は「同じコード」と「同じ知識」を混同していたせいだと気づきました。

共通化すると何が起きるか

自分の場合、フロントエンドでCSVをアップロードしてAPIに送る処理でした。ユーザー設定、部署設定、部署担当者の設定などがあって、どれも「ファイルをアップロードして、結果を受け取って、結果によってインポート処理に進む」という流れは同じです。共通処理としてまとめました。

コードを共通化すると、依存の矢印がそこに集中します。3つの画面すべてが同じCSVアップロード処理を呼び出しているので、共通処理を変更すると3つの画面すべてに影響が波及します。共通化した分だけ、変更の影響範囲が広がっているわけです。

便利だから使い回した結果、多くの箇所が依存する「中心部」になっていました。変更の影響が大きすぎて、気軽に触れなくなっていたんです。

「同じコード」と「同じ知識」は違う

共通化が問題になるのは、まとめるべきでないものをまとめたときです。

本を読んで一番腑に落ちたのが、DRYの「繰り返すな」は「同じコードを繰り返すな」ではなく、「同じ知識を繰り返すな」 という意味だったということです。

見た目が同じコードでも、変わる理由が違うなら、それは別の知識です。まとめてしまうと、片方の理由で変更したときにもう片方が巻き添えを食います。

CSVアップロードの例で考えると、処理の流れは同じでも、設定画面ごとに求められるバリデーションやエラーハンドリングは違います。ユーザー設定ではメールアドレスの重複チェックが必要だし、部署設定では親部署の存在チェックが必要です。変わる理由が違うのに「どれもCSVアップロードだから」とまとめてしまうと、ユーザー設定側の要件で共通処理を変えたときに、部署設定側の挙動まで変わってしまいます。自分のケースもまさにこういう状態でした。

共通化する前に「これは同じ理由で変わるものか？」と問うべきでした。同じ理由で変わるなら共通化は正しい判断です。でも変わる理由が違うなら、コードが似ていてもまとめない方がいいと思います。

違う部分を外から渡す

とはいえ、実際には「8割は同じで2割だけ違う」という状況がよくあります。共通化しないとコードが重複しますし、共通化すると変更しにくくなります。このジレンマに対して、本で紹介されていた手法のひとつが「依存性注入（Dependency Injection）」です。

CSVアップロードの例で考えてみます。自分が最初に書いていたのは、こういうコードでした。

class CsvUploader {
  async upload(file: File, type: "user" | "department" | "staff") {
    if (type === "user") {
      // メールアドレスの重複チェック
    } else if (type === "department") {
      // 親部署の存在チェック
    } else if (type === "staff") {
      // 所属部署の存在チェック
    }

    const result = await api.post("/import", file);
    return result;
  }
}

共通処理の中にif分岐でバリデーションを埋め込んでいます。設定の種類が増えるたびにここに分岐が増えていきますし、ユーザー設定のバリデーションを変えたいだけなのに共通処理を触ることになります。

依存性注入では、違う部分を外から渡せるようにします。

class CsvUploader {
  constructor(private validate: (file: File) => ValidationResult) {}

  async upload(file: File) {
    const validation = this.validate(file);
    if (!validation.ok) return validation;

    const result = await api.post("/import", file);
    return result;
  }
}

CsvUploader はバリデーションの具体的な中身を知りません。「ファイルを受け取ってAPIに送る」という共通の流れだけを持ち、どうバリデーションするかは呼び出し元が決めます。ユーザー設定のバリデーションを変えても、共通処理には一切触れずに済みます。

自分のケースでも、こうしていれば共通処理にif分岐を足す必要はなかったし、似たようなコードの別クラスを作ることもなかったと思います。

まとめ

「同じコードがあるから共通化しよう」という判断基準だけでは、変わる理由が違うものまでまとめてしまう危険があります。共通化する前に問うべきは「これは同じ理由で変わるものか？」でした。

正直なところ、まだ実践で使いこなせている実感はありません。ただ、共通化する前に立ち止まれるようになったのは確かです。なんでも共通化、から、その処理の意味を考えるようにしていこうと思います。

参考

• Sandi Metz著『オブジェクト指向設計実践ガイド ～Rubyでわかる 進化しつづける柔軟なアプリケーションの育て方』技術評論社