かとじゅんの技術日誌

技術の話をするところ

生成だけではなく複製もファクトリに任せたほうがよい

Java DDD

DDDで設計を始めると不変条件を維持するために、エンティティなどの可変オブジェクトの複製を行うことがよくあります。
Javaの場合は、Cloneableインターフェイスを実装して、実装型に応じた複製インスタンスを返すcloneメソッドを作る。以下のような感じ。

public class Employee implements Cloneable {

    private String name;
    // setter, getter 省略  

    @Override
    public Employee clone() {
        try{
            return (Employee)super.clone();
        } catch (CloneNotSupportedException e){
            throw new Error(e);
        }
    }

}

このcloneメソッドは便利なので普通に使っていたのですが、最近、簡単に使ってよいのだろうか、本来の責務としてずれていないかと思うようになってきました。ということで、今更ながらいろんな文献ひっくり返しました(^^ゞ
Joshua Blochが「Effective Java 第二版」「項目17 継承のために設計および文書化する、でなければ継承を禁止する」P88 で こういうこと言っています。

継承を可能にするためには、クラスが従わなければならない制約が多少あります。直接的、間接的のどちらであってもコンストラクタは、オーバーライド可能なメソッドを呼び出してはいけません。

Effective Java 第2版 (The Java Series)

Effective Java 第2版 (The Java Series)

パズルでアンチパターンを学ぶ

この意味を理解するのにちょうど良いサンプルは、「JAVA PUZZLERS 罠、落とし穴、コーナーケース」の「第６章 洒落たパズラー」の「パズル51:何が言いたいの? (What's the Point?)」が参考になります。パズルのソースコードは以下。ColorPointのmainメソッドで何が表示されると思いますか？
[rakuten:book:11547432:detail]

// 座標を表すクラス
public class Point {
	private final String name; // 生成時にキャッシユされる
	protected final int x, y;
	// setter,getter省略

	public Point(int x, int y) {
		this.x = x;
		this.y = y;
		name = makeName();
	}

	protected String makeName() {
		return "[" + x + "," + y + "]";
	}

	@Override
	public final String toString() {
		return name;
	}
}

// 色を持つPointクラス
public class ColorPoint extends Point {

	public static void main(String[] args) {
		System.out.println(new ColorPoint(4, 2, "purple"));
	}

	private final String color;

	public ColorPoint(int x, int y, String color) {
		super(x, y);
		this.color = color;
	}

	@Override
	protected String makeName() {
		return super.makeName() + ":" + color;
	}
}

結果は以下です。

[4,2]:null

Pointクラスのコンストラクタでオーバーライド可能なmakeNameメソッドを呼び出しています。サブクラスであるColorPointクラスはそのmakeNameメソッドをオーバーライドしましたが、makeNameメソッドはthis.colorが初期化されるより前に呼ばれてしまいます。だから結果が"[4,2]:null"なのです。
パズルでは以下のように指摘しています。

型に対応したデフォルト値の状態であっても、final フィールドに値が代入される前に、そのフィールドの値を読み出すととが可能であることを、このパズルは示しています。

これは注意しなければなりません。

cloneメソッドはコンストラクタとかなり似た振る舞いをする

「Effective Java 第二版」「項目17 継承のために設計および文書化する、でなければ継承を禁止する」P88では、cloneメソッドはコンストラクタとかなり似た振る舞いとなると言っています。

継承のために設計されているクラスでCloneableあるいはSerializableを実装すると決めたならば、cloneメソッドとreadObjectメソッドはコンストラクタとかなり似た振る舞いをするので、同様の制限が適用されることを認識しなければなりません。その制限とは、cloneおよびreadObjectは、オーバーライド可能なメソッドを、直接的、間接的のどちらであっても呼び出してはいけないということです。

そしてパズルでもこれらのメソッドは擬似コンストラクタだと言っています。

(これらのメソッドは、コンストラクタを呼び出すととなくオブジェクトを生成するので、疑似コンストラクタと呼ばれます。)

なので、cloneメソッドでも、先ほどのオーバーライド可能なメソッドを呼び出すのは避けたほうがよいとのこと。

cloneメソッド内部でオーバーライドしたメソッドを呼んでみる

それでは、実際に車のオブジェクトで考えてみます。
車の抽象クラスであるAbstractCarクラスのcloneメソッドでは、super.clone()以外にtiresフィールドはマップです。つまり、同一インスタンスで状態変更可能な可変オブジェクトです。複製する時は新たなマップのインスタンスを作る必要があるので、cloneTiresメソッドを使って複製を作っています。そして、tiresフィールドのマップに格納する要素型もTireクラスも可変オブジェクトです。つまり、cloneTiresメソッドで複製する際は要素型のインスタンスの複製を作成し、それを保持する新たなマップのインスタンスを作る必要があります。

public abstract class AbstractCar implements Cloneable {
	private final String id;
	// タイアのマップ
	private Map<Position, Tire> tires = new HashMap<Position, Tire>();

	public AbstractCar(String id) {
		this.id = id;
	}

	// srcを複製するメソッド
	protected Map<Position, Tire> cloneTires(Map<Position, Tire> src) {
		Map<Position, Tire> result = new HashMap<Position, Tire>();
		Set<Entry<Position, Tire>> entrySet = src.entrySet();
		for (Entry<Position, Tire> entry : entrySet) {
			result.put(entry.getKey(), entry.getValue().clone());
		}
		return result;
	}

	// cloneメソッド
	@Override
	public AbstractCar clone() {
		try {
			AbstractCar result = (AbstractCar) super.clone();
			// tiresフィールドをディープコピー
			result.tires = cloneTires(tires);
			return result;
		} catch (CloneNotSupportedException e) {
			throw new Error(e);
		}
	}

	public final String getId() {
		return id;
	}

	public void addTire(Tire tire) {
		tires.put(tire.getLocalId(), tire);
	}

	public Set<Tire> getTires() {
		Set<Tire> result = new HashSet<Tire>();
		for (Tire t : tires.values()) {
			result.add(t.clone());
		}
		return result;
	}

	// hashCode, equalsメソッドは省略


}

AbstractCarクラスを継承するLegacyクラスでcloneTiresメソッドをオーバーライドして、ConcurrentHashMapの複製を作っている。かのようですが、要素型の複製を忘れているので不変条件が破壊されています。つまり、legacy.clone() してもなお要素のオブジェクトへの参照は共有していることになるわけです。もう一つの問題は、tiresフィールドは本来はfinalフィールドにしたいのですが、cloneメソッドのために再代入を許可しないといけないので、非finalフィールドとしています。

public class Legacy extends AbstractCar {

	public Car(String id, Engine engine) {
		super(id);
		this.engine = engine;
	}

	private Engine engine;

	// こんなオーバーライドをさせると危険。
	@Override
	protected Map<Position, Tire> cloneTires(Map<Position, Tire> src) {
		Map<Position, Tire> result = new ConcurrentHashMap<Position, Tire>();
		result.putAll(src); // 要素のcloneが抜けた!
		return result;
	}

	@Override
	public Car clone() {
		Car result = (Car) super.clone();
		result.engine = engine.clone();
		return result;
	}

	public void setEngine(Engine engine) {
		this.engine = engine.clone();
	}

	public Engine getEngine() {
		return engine.clone();
	}
}

こういう問題を回避するには、cloneTiresメソッドをfinalとしてそもそもオーバーライド不可能にするという方法です。

public abstract class AbstractCar implements Cloneable {
	// ...
	// finalメソッドにして、そもそもサブクラスでオーバーライドさせない
	protected final Map<Position, Tire> cloneTires(Map<Position, Tire> src) {
		Map<Position, Tire> result = new HashMap<Position, Tire>();
		Set<Entry<Position, Tire>> entrySet = tires.entrySet();
		for (Entry<Position, Tire> entry : entrySet) {
			result.put(entry.getKey(), entry.getValue().clone());
		}
		return result;
	}
	// ...
}

オーバーライド可能なメソッドを呼ばない、呼ばせない工夫はもちろんやるべきです。
しかし、そもそも ディープコピーを注意深く扱うことはモデルにとって重い責務ではないかと思い始めました。擬似コンストラクタとも呼ばれている所以がわかる気がします。DDDではオブジェクの生成にはファクトリメソッドやファクトリクラスを使うことを推奨しますが、複製もファクトリなのかもしれません。

エリック・エヴァンスのドメイン駆動設計 (IT Architects’Archive ソフトウェア開発の実践)

エリック・エヴァンスのドメイン駆動設計 (IT Architects’Archive ソフトウェア開発の実践)

cloneメソッドではなく「自前のコピーメソッド」を利用する

最近買った「Java ルールブック」の「3.1.5 Cloneable#clone()を使わず、目前のコピーメソッドを利用する」P156 を読んでなるほどと思いました。

Javaルールブック　?読みやすく効率的なコードの原則

Javaルールブック　?読みやすく効率的なコードの原則


cloneメソッドはシャローコピーなので、そこで可変オブジェクトなどを考慮してディープコピーすることを忘れたり、非finalによる初期化漏れの心配もあります。完全なオブジェクトを作るという意味からすると、finalをつけたいところです。そういう抜け漏れによって思わぬ不具合を発生させてしまうかもしれません。なので、別のコピーメソッドで複製インスタンスを作ったほうがよいという考え方です。*1

	@Override
	public AbstractCar clone() {
		try {
			AbstractCar result = (AbstractCar) super.clone();
			// ディープコピーをし忘れる可能性がある。
			// tiresフィールドが非finalによる代入漏れの心配もある。
			return result;
		} catch (CloneNotSupportedException e) {
			throw new Error(e);
		}
	}

この指針に従って「自前のコピーメソッド」を作ってみました。

public class Car {
	private final String id;
	private final Map<Position, Tire> tires = new HashMap<Position, Tire>();

	public Car(String id) {
		this.id = id;    
	}

	public static Car copy(Car car) {
		Car result = new Car(car.getId(), Engine.copy(car.getEngine()));
		Set<Entry<Position, Tire>> entrySet = car.tires.entrySet();
		for (Entry<Position, Tire> entry : entrySet) {
			result.tires.put(entry.getKey(), Tire.copy(entry.getValue()));
		}
		return result;
	}

}

当然、自クラスだけで完結するので単純にコードを書けますね。このパターンでは、tiresフィールドをfinalフィールド化することができるようになりました。これはスレッドセーフなモデルを考える上で都合がよいですね。

複製を「擬似生成」と考えるとファクトリの責務になるのではないか

複製を作るメソッドはコンストラクタと似た振る舞いをすると考えると、モデル側ではCloneableインターフェイスの実装とcloneメソッドは廃止して、ファクトリメソッドや、ファクトリクラスに複製処理を委譲したほうがよいかもしれません。
Legacyクラスであれば、以下のようなLegacyFactoryクラスを実装してはどうかという話しですね。つまり、「自前のコピーメソッド」というのを拡大解釈して、複製時の不変条件を守るということを考えると、DDDのファクトリパターンでオブジェクトの複製を実現した方がよいのではないかという視点です。

public final class LegacyFactory {

	private final EngineFactory engineFactory = new EngineFactory();
	private final TireFactory tireFactory = new TireFactory();

	// 通常の生成
	public Legacy newLegacy(String id, Engine engine) {
		return new Legacy(id, engine);
	}
	
	// 擬似生成(複製)を行う 既存のLegacyを元に同じ値を持つ新しいLegacyを作る
	public Legacy newLegacy(Legacy car) {
		Legacy legacy = new Legacy(car.getId(), engineFactory.newEngine(car
				.getEngine()));
		Set<Entry<Position, Tire>> entrySet = car.tires.entrySet();
		for (Entry<Position, Tire> entry : entrySet) {
			legacy.tires.put(entry.getKey(),
					tireFactory.newTire(entry.getValue()));
		}
		return legacy;
	}
}

AbstractCarのcloneメソッドの戻り値は、AbstractCarです。そして、そのサブクラスのLegacyのcloneメソッドの戻り値はLegacyです。つまり、cloneメソッド内部で自クラスのキャストがどうしても発生しますが、ファクトリを導入すればそういった問題はそもそも回避できます。

まとめ

まとめると以下ということになると思います。

  • 複製というのはそもそもモデルの責務ではなく、ファクトリのメソッドではないか。
  • スレッドセーフを考慮するならば、できる限りモデルのフィールドはfinalフィールドにしたほうがよい。

当然、ファクトリにすることによるデメリットもあると思います。
多分、cloneメソッドより抽象化が難しくなるでしょう。それと、実装型を知らない前提では、インスタンスの複製ができない。そういう場合はcloneメソッドしかないですね。
cloneの使いどころとしては、こちらも参照されたい。
Javaのcloneは悪者か？ - 都元ダイスケ IT-PRESS
自分が作っているモデルでは、実装型を知らないということはないので、上記の二つを考えるとファクトリのほうが自然ではないかなと思いました。みなさんは、どう思いますか？

*1:「Effective Java第二版」の「項目11 cloneを注意してオーバーライドする」と関連する考え方だと思います。