Primitive Obsession

コードスメルの1つであるPrimitive Obsessionの何が不味くて、どういう対策がありうるのか、というのを収集する。

Primitive Obsessionのもたらすマズイこと

書籍から

Primitive Obsessionに言及している書籍から抜き出してみる。

『リファクタリング』より

デメリットはハッキリとは書いていない。3つほど例が挙げられている。

金額をタダの数値として扱う

単位を無視した物理量の計算をしてしまう

if (a < upper && a > lower) みたいなコードがたくさん散りばめられる。

『Programming with Types』

与えられた単位を誤解して、それが異なる計算をしてしまう。例) メートル法の計算ロジックに、ヤードの単位の数値を渡せてしまう。

これはリファクタリングの2つめと同じ

『The Art of Agile Development, 2nd Edition』

コードの重複が増える

これはリファクタリングの3つめと同じ

『Learning Domain-Driven Design』

検証ロジックが重複しがち

値が使用される前に検証ロジックを呼び出すことを強制するのが困難

『Programmer's Brain』

コードの認知負荷が高まる

マズイことまとめ

1. (検証ロジック含む)同じ処理があちこち分散してしまう原因になる

2. 検証の責務を呼び出し側に強制できない

3. プリミティブな値以外にもつべきメタデータ(単位など)が抜け落ちる

4. メソッドに多くの引数が並ぶことにつながり、読むのが大変だし、取り違えの元になる

Primitive Obsessionの対策

例題

会議室予約システムのようなものを題材に、Primitive Obsessionな例を考える。Reservation型のプロパティfrom, toは、予約時間を表す。また、chargeは利用料金を表す。以下に、示す通り予約しようとした時間帯に予約が入っているか調べるのに、if条件式を書く必要があり、似たような処理がいろんなユースケースにあると、コードの重複が発生する。これが『リファクタリング』に載っているPrimitive Obsessionのコードスメルの1つ。

code:ts

interface Reservation {

roomId: string;

charge: number;

currency: string;

from: Date;

to: Date;

}

const reservations: Reservation[] = [];

reservations.push({

roomId: "123",

charge: 1000,

from: new Date(2022, 8, 15, 10),

to: new Date(2022, 8, 15, 11),

});

const roomId = "123";

const from = new Date(2022, 8, 15, 9);

const to = new Date(2022, 8, 15, 11);

// 指定した時間帯に予約が入っていないか確認し、空いていれば予約する

if (reservations.every(reservation => reservation.roomId !== roomId

|| to.getTime() < reservation.from.getTime()

|| from.getTime() > reservation.to.getTime())) {

// 予約処理

}

同じ処理があちこち分散しないように

似たような処理のコード重複を何とかしようとすれば、単に同じ処理を作らないようにロジックを共通化する、という話になる。

例えば日付のFrom~Toをセットで扱うクラスを用意し、ここに時間帯に入っているか判定する処理などを持たせる。

code:ts

class DateRange {

private readonly from: Date;

private readonly to: Date;

constructor(from: Date, to: Date) {

this.from = from;

this.to = to;

}

contains(another: DateRange): boolean {

return another.to.getTime() >= this.from.getTime()

&& another.from.getTime() <= this.to.getTime();

}

}

interface Reservation {

roomId: string;

charge: number;

currency: string;

period: DateRange;

}

こうしたコード重複の解消が目的であれば、Value Objectである必要はない。し、イミュータブルにクラス設計する必然性もない。

『リファクタリング』だと"Replace Primitive with Object"がこの目的でのパターン。

検証の責務を呼び出し側に強制するには?

型安全の議論から、なぜ独自型を作るのか?

検証の責務を呼び出された側じゃなくて、呼び出し側に持ってくる。

code:java

// denominatorが0の時は計算できないので、Optional#emptyが返る

Optional<Integer> devide(int numerator, int denominator);

// 呼び出し側でdenominatorが非ゼロであることを保証する。そのためにNonZero型を用意する。

int devide(int numerator, NonZero denominator);

これにより、事前条件を満たすことを型によって呼び出し側に強制することができる。

プリミティブな値以外にもつべきメタデータ(単位など)が抜け落ちる

通貨単位があることを見落として、chargeだけで料金計算ロジックが組まれると、確かに大きな問題となる。

これを解消する目的では、MonetaryAmountクラスを作って単位を内包するとよい。これはある変換可能な単位群での数量を表すものになるので、値で比較できると何かと便利なので、Value Objectとして実装するのが適した箇所になる。

メソッドに多くの引数が並ぶことにつながり、読むのが大変だし、取り違えの元になる

『リファクタリング』だと"Introduce Parameter Object"がこの目的に繋がる。意図しない書き換えが起こらないように、Parameter Objectにはイミュータブルな設計が望まれる。

Value ObjectがPrimitive Obsession対策たりえるのか?

Value Objectを最大限拡張利用しているIDDDでは以下6つを特徴としている。

計測/定量化/説明

不変性

概念的な統一体

交換可能性

等価性

副作用のない振る舞い

Primitive Obsession Obsession