はじめに
前回同様、データベーススペシャリストの問題を使ってDDDでサンプルプロググラムを作ったのリファクタリングした記事になります。
ドメイン駆動設計でデータベーススペシャリストの問題を使ってサンプルプログラムを作ってみた
DDDでデータベーススペシャリストの問題を使ってサンプルプログラム作成 改善その1
今回は、「入金する」のエンドポイントに関連する箇所をリファクタリングしてみました。
ソースコードは こちら
本記事投稿時のバージョンはタグ1.2になります。
多段階抽選で抽選の当落判定をリファクタリング
最初に投稿した記事にて
多段階抽選で抽選の当落をエントリの外側から判定しているのはロジックがApplicationに漏れる原因となりますし、なにより段階が増えたときに後で後悔することになります。2段階とは書いてませんし、ぶっちゃけ2段階に増やしたものはもう一段増やしてくれって要望がすぐに出ます。
というコメントをいたいだいていました。
こちらをトライしてみました。
まず、抽選の参加申込の抽選結果を多段階抽選の結果も含めて取得するようにして、それをファーストクラスコレクションに登録し、ファーストクラスコレクションから当選しているかどうかを取得できるようにしてみました。
public class LotteryEntryResults {
private List<LotteryEntryResult> list;
public LotteryEntryResults(List<LotteryEntryResult> list) {
this.list = list;
}
/**
* 当選しているかどうかを返す.当選している場合、trueを返す.
*/
public boolean winning() {
for (LotteryEntryResult result : list) {
if (result.lotteryResult == LotteryResult.winning) {
return true;
}
}
return false;
}
}
@Override
public LotteryEntryResults findLotteryEntryResults(
FestivalId festivalId,
MemberId memberId,
EntryId entryId) {
List<LotteryEntryResult> resultList = new ArrayList<>();
EntryId targetEntryId = entryId;
while (true) {
LotteryEntryResult lotteryEntryResult = lotteryEntryResultMapper.selectLotteryEntryResult(
festivalId, memberId, targetEntryId);
if (lotteryEntryResult == null) {
break;
}
resultList.add(lotteryEntryResult);
EntryDto entryDto = entryMapper.selectEntry(festivalId, targetEntryId);
EntryId followingEntryId = entryDto.followingEntryId();
if (followingEntryId == null) {
break;
}
targetEntryId = followingEntryId;
}
return new LotteryEntryResults(resultList);
}
再帰処理を使ってみてはどうかとコメントいただいていたのですが、SQLで再帰処理をやったことがなかったので、今回は泥臭くやってみました
これにより多段階抽選で抽選の当落判定をアプリケーション層からドメイン層に移動することができました。
また、3段階以上の多段階抽選にも対応できるようになりました。
リファクタリング前
Entry entry = entryRepository.findEntry(festivalId, application.entryId());
if (entry.isLotteryEntry()) {
// 対象のエントリが抽選なら当選しているかを確認する
LotteryEntryResult entryResult = lotteryEntryResultRepository.findLotteryEntryResult(
festivalId, memberId, entry.entryId());
if (entryResult.lotteryResult() == LotteryResult.failed) {
EntryId followingEntryId = ((LotteryEntry)entry).followingEntryId();
if (followingEntryId == null) {
throw new BusinessErrorException("対象の大会には当選していません");
} else {
LotteryEntryResult followingEntryResult =
lotteryEntryResultRepository.findLotteryEntryResult(
festivalId, memberId, followingEntryId);
if (followingEntryResult.lotteryResult() == LotteryResult.failed) {
throw new BusinessErrorException("対象の大会には当選していません");
}
}
}
}
リファクタリング後
Entry entry = entryRepository.findEntry(festivalId, application.entryId());
if (entry.isLotteryEntry()) {
// 対象のエントリが抽選なら当選しているかを確認する
LotteryEntryResults lotteryEntryResults =
lotteryEntryResultRepository.findLotteryEntryResults(
festivalId, memberId, entry.entryId());
if (!lotteryEntryResults.winning()) {
throw new BusinessErrorException("対象の大会には当選していません");
}
}
if文が減ってすっきりしました
ポイントを利用する箇所のリファクタリング
ポイント利用の処理についても
MemberPoints はファーストクラスコレクションにしたのはよいと思いますが for の中が長いなぁという印象ですね。
MemberPoints の for の中にしても大抵はコレクションメンバーである MemberPoint 自体に持たせたほうがすっきりするものが多いと思いました。
ポイント残高は、Pointクラスに持たせた方がいいメソッドのような気がしますね。
あと有効期限もplusYears(1)でハードコーディングしてますが、これもPointクラスに持たせた方がよさそう。
などのコメントをいただき、いただいたコメントを参考にリファクタリングしてみました。
リファクタリング前
public class MemberPoints {
private List<MemberPoint> list;
/**
* 引数のポイントが使えるかどうかを判定し、有効期限の近いポイントから使用する.
* なお、保持する MemberPoint オブジェクトの状態を変更する。
*/
public void usePoints(LocalDate paymentDate, PointAmount pointAmount) {
// この値がゼロになるまでこれまで付与されたポイントからポイントを使用していく
BigDecimal x = pointAmount.value();
for (MemberPoint memberPoint : list) {
// 有効期限のチェック
LocalDate expirationDate = memberPoint.givenPointDate().plusYears(1);
if (paymentDate.compareTo(expirationDate) > 0) {
continue;
}
// ポイント残高のチェック
BigDecimal availableUsePoint = memberPoint.givenPoint().value()
.subtract(memberPoint.usedPoint().value());
if (availableUsePoint.compareTo(BigDecimal.ZERO) == 0) {
continue;
}
if (availableUsePoint.compareTo(x) <= 0) {
memberPoint.use(availableUsePoint);
x = x.subtract(availableUsePoint);
} else {
memberPoint.use(x);
x = BigDecimal.ZERO;
break;
}
}
// 有効期限の近いものからポイントを使用していき、利用したいポイントに満たなかった場合エラー
if (x.compareTo(BigDecimal.ZERO) > 0) {
throw new BusinessErrorException("ポイント数が不足しています");
}
}
}
リファクタリング後
public class MemberPoints {
private List<MemberPoint> list;
/**
* 引数のポイントが使えるかどうかを判定し、有効期限の近いポイントから使用する.
* なお、保持する MemberPoint オブジェクトの状態を変更する。
*/
public void usePoints(LocalDate paymentDate, PointAmount usePointAmount) {
PointAmount pointBalance = pointBalance(paymentDate);
if (pointBalance.value().compareTo(usePointAmount.value()) < 0) {
throw new BusinessErrorException("ポイント数が不足しています");
}
// この値がゼロになるまでこれまで付与されたポイントからポイントを使用していく
BigDecimal x = usePointAmount.value();
for (MemberPoint memberPoint : list) {
// 有効期限のチェック
if (memberPoint.hasPassedExpirationDate(paymentDate)) {
continue;
}
// ポイント残高のチェック
PointAmount availablePoint = memberPoint.availablePoint(paymentDate);
if (!availablePoint.isPositive()) {
continue;
}
if (availablePoint.value().compareTo(x) <= 0) {
memberPoint.use(availablePoint);
x = x.subtract(availablePoint.value());
} else {
memberPoint.use(new PointAmount(x));
break;
}
}
}
private PointAmount pointBalance(LocalDate paymentDate) {
PointAmount result = new PointAmount(BigDecimal.ZERO);
for (MemberPoint memberPoint : list) {
PointAmount availablePoint = memberPoint.availablePoint(paymentDate);
result = result.add(availablePoint);
}
return result;
}
}
public class MemberPoint implements Entity {
private MemberId memberId;
private LocalDate givenPointDate;
private PointAmount givenPoint;
private PointAmount usedPoint;
/**
* 引数で指定した対象日で利用可能なポイント数を返す.
*/
PointAmount availablePoint(LocalDate targetDate) {
if (targetDate.compareTo(expirationDate()) > 0) {
return new PointAmount(BigDecimal.ZERO);
}
BigDecimal result = givenPoint.value().subtract(usedPoint.value());
return new PointAmount(result);
}
/**
* 有効期限を返す.
*/
LocalDate expirationDate() {
return givenPointDate.plusYears(1);
}
/**
* 有効期限を過ぎていないかどうかを返す.有効期限を過ぎている場合、trueを返す.
*/
boolean hasPassedExpirationDate(LocalDate paymentDate) {
return paymentDate.compareTo(expirationDate()) > 0;
}
}
MemberPoint
クラスに利用可能なポイント数を返すメソッドと、有効期限、有効期限を過ぎていないかを返すメソッドを作ることにより、すこしだけ、MemberPoints
クラスのusePoints
メソッドをすっきりさせることができました。
さいごに
ドメイン駆動設計というより、オブジェクト指向らしく作るってことなんですかねぇ。
両方とも本当に難しいです
特に私の場合、戦術的DDDしかできていないので、「境界付けられたコンテキスト」等は全然わかっていないです…
いつかは、戦略的DDDについてもアウトプットできるといいなぁ…
また、もう一つ「抽選結果を登録する」のエンドポイントがあったのですが、こちらはあまり良い案が思いつかず、今回は断念しました。
(そもそものエンドポイントの仕様が良くなかった気がしています)
今回、このデータベーススペシャリストの問題を使ってサンプルのプログラムを作ってみて、経験値上がった気がしています。
また、別の年度の問題を使って挑戦して今回より少しは成長したアウトプットができたらいいなと思っています。
ここまで読んでくださり、ありがとうございました。
みなさんからのコメントとても嬉しく、学びもたくさんあるので、コメントいただけると幸いです。