はじめに

この記事について

以下の記事について、感じたことをメモに残しておきます。

記事内で紹介されている「Google Testing Blog」の記事に書かれているコードは、パッと見では共通化しないほうがいいかも？と思ったんですが、少し考えて、このケースでは共通化してもいいのではないか、と思い直したので、なんでそう思ったのか、言語化を試みます。

DRY原則は有名ですが、それと同様に、早まった共通化（あるいは抽象化）はアンチパターンである、というのはあるていど浸透しているのかな、という印象はあります（ブコメでも肯定的なコメントが多いようですし）。

DRY原則とは

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

Andrew, Hunt; Thomas David. Pragmatic Programmer, The: From Journeyman to Master (p. 431). Pearson Education. Kindle Edition.

私は「達人プログラマー」で初めて知りました。手元に原著しかなかったので、英語での引用ですが、訳すと「システムの中で、あらゆる知識は唯一で、明確な、信頼に足る表現を持たなければならない。」みたいな感じでしょうか。

同じ表現が複数箇所にある場合、修正の必要が発生したときに、すべてを直さなくてはならないため、修正漏れが発生するリスクを抑えるために、一箇所にまとめておきましょう、という提言です。

共通化したほうがいいケースとしないほうがいいケース

さて、本題ですが、まず、件の記事に書かれたコードを引用してみます（少しフォーマットは変えてあります）。

共通化したコード

class DeadlineSetter:
 def __init__(self, entity_type):
  self.entity_type = entity_type

 def set_deadline(self, deadline):
   if deadline <= datetime.now():
    raise ValueError(“Date must be in the future”)

task = DeadlineSetter(“task”)
task.set_deadline(datetime(2024, 3, 12))

payment = DeadlineSetter(“payment”)
payment.set_deadline(datetime(2024, 3, 18))

共通化していないコード

def set_task_deadline(task_deadline):
  if task_deadline <= datetime.now():
    raise ValueError(“Date must be in the future”)  

def set_payment_deadline(payment_deadline):
  if payment_deadline <= datetime.now():
    raise ValueError(“Date must be in the future”)  

set_task_deadline(datetime(2024, 3, 12))

set_payment_deadline(datetime(2024, 3, 18))

たしかに、タスクの期日と支払いの期日は別物なので、共通化しないほうがいいと感じます（本筋とあまり関係ないですが、なぜ共通化したほうはクラスベースで共通化していないほうは関数ベースなんでしょう？理由はわからないですが、以下ではクラスベースで記述します）。

しかし、上のコードは Setter/set という名前に反して、条件（期日が過去である）を満たした場合は例外を投げる、という処理になっています。つまりこれの実態はバリデーションとみなせます（あるいは Specification パターン）。

「『期日』を過去の日付に設定できてはならない」というのは仕様です。そして、「タスクの期日と支払いの期日は『ともに』過去の日付に設定できてはならない」というのも仕様です。このシステムが「タスクのみ期日を過去の日付に設定できる」（それの是非は置いといて）という仕様を持つのであれば、それに応じた表現が必要になるでしょう。

しかし、上のコードではそれを読み取ることはできません。いずれのコードでも単なる日付オブジェクトを入力に受け取っているので、関数としてはそれがタスクの期日なのか支払いの期日なのかは知る手段がありません。

もし仮に「期日を過去の日付に設定できてはならない」という仕様だけがあり、システムを組むのであれば、私ならその処理は共通化するなぁ、と思いました。そして、もし将来「◯◯の期日は過去の日付に設定できる」という新たな仕様が発生した時点で、処理を分割するでしょう。

上のコードをアレンジして使うとしたらこんな感じにするでしょう。

import datetime as dt

class DeadlineValidator:
    def validate(self, deadline):
      if not isinstance(deadline, dt.datetime):
          raise ValueError('Deadline must be a datetime object')
      if deadline < dt.datetime.now():
          raise ValueError('Deadline must be in the future')

class Task:
    def __init__(self, name, deadline):
      deadline_validator = DeadlineValidator()
      deadline_validator.validate(deadline)
      self.name = name
      self.deadline = deadline

task = Task('task', dt.datetime.now() - dt.timedelta(days=1))
# ValueError: Deadline must be in the future

class Payment:
    def __init__(self, name, deadline):
      deadline_validator = DeadlineValidator()
      deadline_validator.validate(deadline)
      self.name = name
      self.deadline = deadline

payment = Payment('payment', dt.datetime.now() - dt.timedelta(days=1))
# ValueError: Deadline must be in the future

もし「タスクのみ期日を過去の日付に設定できる」という新たな仕様が発生したら、バリデーションを外せばいいので、仕様変更に対してコードの変更が対になってわかりやすくなる予感がします。

では、逆に共通化しないほうがいいケースはどんなケースかな、と考えてみました。

同じくバリデーションで考えてみると、ものすごく単純な例ですが、たとえば、ある要素の最大文字数と別の要素の最大文字数が同じだとしても、それはたまたまである可能性が高いでしょう。

私の大好きな「オブジェクト指向設計実践ガイド」の著者である Sandi Metz さんのブログ記事を紹介します。

sandimetz.com

誤りは共通化（本記事では「抽象化」）それ自体ではなく以下の部分だと思うんですよね。

Programmer B feels honor-bound to retain the existing abstraction,

どちらかというと、仕様変更が生じたときに安易に引数を増やしたり、条件分岐を増やしたりするほうを防いだほうが、保守性の観点では有用なのかなと思います。

記事中にも

Do the following: 1. Re-introduce duplication by inlining the abstracted code back into every caller. 2. Within each caller, use the parameters being passed to determine the subset of the inlined code that this specific caller executes. 3. Delete the bits that aren't needed for this particular caller.

とあります。ただ、たしかに、

Existing code exerts a powerful influence.

という側面はあるでしょう。心理的には既存のコードをなるべく変えずに仕様変更に対応したい、と思うときは私自身にもあります。しかし、それを避けるためという理由で共通化を避けるというのは、どうしても正しい選択に思えないのです。

「早すぎる共通化」に関してよく言われるのは「偶然同じになっているコードは共通化してはいけない」という主張です。しかし、偶然であろうが必然であろうが、その時点で将来に渡って同一性を維持できるかどうかはわからないですし、すべての処理を共通化しないという選択もできないので、個人的には、コードを書く時点で同じ概念であれば共通化してもいいのではないか、と思います。

2アウトルール（二箇所重複で共通化する）、3アウトルール（三箇所重複で共通化）なんてのもありますが、私はどちらかというと2アウト派ではあるものの、強いこだわりはとくになく、気づいたら共通化する、といった感じです。

その一方で、共通化しなければならないとも思いません。

仕様変更が起きたときに、それに対してコードが追従することができれば、手段はなんでもいいはずで、複数箇所を同時に修正する必要があったとしても、それは検索して探せばいいわけです。実際、部分的に共通であっても、それらをすべて共通化することは現実的ではないので、重複コードは必ずあると思っておいたほうがよさそうです。コピペなら、コピペするときに重複することは確実にわかりますが、たまたま同じ処理を書いてしまう、ということを防ぐのは難しいでしょう（静的解析ツールがあるていど捕捉してくれるとはいえ）。