まとめるか分割か〜ソフトウェア設計のきほん〜

はじめに

この記事ではソフトウェア設計において

適切にまとめたり分割できなければどうなるか
どのようにまとめたり分割するか
適切にまとめたり分割することで何が得られるか

についてまとめました。

動画でも解説してるのでご覧ください。

まとめ過ぎると読みにくく・再利用できず・テストしずらくなる

買い物をする際に、

付与されるキャンペーンポイント付与について考えてみましょう。

キャンペーンポイント付与でやること

・購入者の会員ランク、キャンペーン付与条件によってキャンペーン付与対象を判定

・購入金額、キャンペーンポイント付与率から付与ポイント数算出

この場合、

皆さんならどのようにコーディングしますか？？

仮に、

キャンペーン付与対象かどうかの判定と付与ポイント数算出を、

1まとめにして「キャンペーン付与」という関数を作ったケースを考えてみましょう。

❌キャンペーン付与対象判定とポイント数算出を1つの関数で行う

class PointCalculate {
    fun execute(
        previousMonthlyPurchaseAmount: Int,
        previousMonthlyPurchaseCount: Int,
        campaignStartDate: LocalDate,
        campaignEndDate: LocalDate,
        pointRate: Double,
        purchaseAmount: Int
    ): Int {

        // キャンペーンが期間内で付与対象かどうか判定
        val today = LocalDate.now()
        val validCampaign = !today.isBefore(campaignStartDate) && !today.isAfter(campaignEndDate)
        if (!validCampaign) return 0

        // 会員ランクが付与対象かどうか判定
        val rank = if (previousMonthlyPurchaseAmount > 10_000) RankType.GOLD
        else if (previousMonthlyPurchaseCount > 10 && previousMonthlyPurchaseAmount > 5_000) RankType.SILVER
        else RankType.BRONZE
        if (RankType.BRONZE == rank) return 0

        // 付与ポイント算出
        return floor(purchaseAmount * pointRate).toInt()


    }
}

enum class RankType {
    BRONZE,
    SILVER,
    GOLD
}

一見、

1箇所にキャンペーン付与に関連するコードがあって

読みやすくわかりやすいようにも思えるかもしれません。

しかし、テストのしやすさはどうでしょう？？

この関数にキャンペーン付与対象判定結果の検証がしたい場合、

この関数に付与ポイント数算出結果の検証がしたい場合、

それぞれどのようにテストコードを書けば良いでしょう？？

1まとまりになっているため通しで全てをテストすることになり、

分岐の掛け合わせパターンを考えると漏れが出そうです。

複数のことをやる関数のテストコード

    @Test
    fun `キャンペーン期間内でランクGold会員にはポイントが付与できる`() {

        val target = PointCalculate()
        val actual = target.execute(
            previousMonthlyPurchaseAmount = 10_001,
            previousMonthlyPurchaseCount = 10,
            campaignStartDate = LocalDate.of(2023, 10, 1),
            campaignEndDate = LocalDate.of(2023, 11, 1),
            pointRate = 0.01,
            purchaseAmount = 1000
        )
        Assertions.assertEquals(10, actual)

    }

    @Test
    fun `キャンペーン期間内でランクBronze会員にはポイントが付与できない`() {

        val target = PointCalculate()
        val actual = target.execute(
            previousMonthlyPurchaseAmount = 10_000,
            previousMonthlyPurchaseCount = 10,
            campaignStartDate = LocalDate.of(2023, 10, 1),
            campaignEndDate = LocalDate.of(2023, 11, 1),
            pointRate = 0.01,
            purchaseAmount = 1000
        )
        Assertions.assertEquals(0, actual)

    }

    @Test
    fun `キャンペーン期間外ではポイントが付与できない`() {

        val target = PointCalculate()
        val actual = target.execute(
            previousMonthlyPurchaseAmount = 10_001,
            previousMonthlyPurchaseCount = 10,
            campaignStartDate = LocalDate.of(2023, 10, 1),
            campaignEndDate = LocalDate.of(2023, 10, 2),
            pointRate = 0.01,
            purchaseAmount = 1000
        )
        Assertions.assertEquals(0, actual)

    }

また、関数は再利用しやすいでしょうか？？

例えばこのような変更が入った場合どうでしょう？？

変更要望

・ECサイトでの購入時だけでなく月次や随時でもキャンペーン付与したい

・キャンペーン付与対象判定はECサイトでの購入時の判定と同一

・付与ポイント数は月の購入金額合算から算出したり固定ポイント数としたい

複数のことを1つの関数にひとまとめにしたため、

一部の判定ロジックだけ再利用したくても、

しづらいですね。

また、関数を使う側の視点に立つと、

関数が中で2つのことをやっていると、

関数が何をやっているのか理解しやすいでしょうか？？

1つの小さな問題を解決するような関数に比べると、

理解しづらいですね。

大きな問題を小さな問題に分割せずに解決しようとすると、

手続的に簡単にコードは書けるかもしれませんが、

このようなことに陥ります。

まとめ過ぎたコードと適切に分割したコード比較

ここからは、

先ほどのキャンペーンポイントをお題にしたサンプルコードを

改善した例を見ていきます。

⭕️キャンペーン付与対象判定、ポイント算出をそれぞれ別々に行う

class Campaign private constructor(val pointRate: Double?) {
    companion object {
        fun create(
            campaignStartDate: LocalDate,
            campaignEndDate: LocalDate,
            pointRate: Double
        ): Campaign {
           // キャンペーン期間中か判定
            val today = LocalDate.now()
            val validCampaign =
                !today.isBefore(campaignStartDate) && !today.isAfter(campaignEndDate)

            if (validCampaign) return Campaign(pointRate)
            return Campaign(null)

        }
    }
}


class Rank private constructor(val rankType: RankType) {
    companion object {
        fun create(previousMonthlyPurchaseAmount: Int, previousMonthlyPurchaseCount: Int): Rank {
            // 会員ランクが何か判定
            if (previousMonthlyPurchaseAmount > 10_000) return Rank(RankType.GOLD)
            if (previousMonthlyPurchaseCount > 10 && previousMonthlyPurchaseAmount > 5_000) return Rank(
                RankType.SILVER
            )
            return Rank(RankType.BRONZE)
        }
    }
}


class IsPointGrantTargetSpec {
    fun execute(rank: Rank, campaign: Campaign): Boolean {
        if (campaign.pointRate == null) return false
        if (RankType.BRONZE == rank.rankType) return false
        return true

    }

}


class PointCalculateSpec {
    fun execute(rate: Double, purchaseAmount: Int): Int {
        return floor(rate * purchaseAmount).toInt()

    }

}

Campaignクラスではキャンペーン期間中か判定

Rankクラスでは会員ランク判定

IsPointGrantTargetSpecではこれらを利用してキャンペーン付与対象判定のみを行う。

PointCalculateSpecではポイント数のみを算出する。

というように今回は修正してみました。

こうすることで、

キャンペーン付与判定の会員ランク絡みの部分の仕様変更時は、

Rankクラスだけを修正すればよく、

ポイント数算出仕様変更時は、

PointCalculateSpecクラスだけを修正すれば良くなります。

また、キャンペーン付与対象判定部分のみを

別のユースケースでも使いまわしたい場合は、

IsPointGrantTargetSpecを使い回せば良くなります。

また、テストコードについて言えば、

例えばRankクラスに対するテストコードは

想定の会員ランクでインスタンスを生成できているかの

検証に特化でき、テストコードも書きやすくなりました。

ポイント算出などいろんなことを気にせずテストコードが書ける

        @Test
        fun `Goldランクの会員を生成できる`() {
            val rank = Rank.create(11_000, 0)

            Assertions.assertEquals(RankType.GOLD, rank.rankType)
        }

        @Test
        fun `Silverランクの会員を生成できる`() {
            val rank = Rank.create(6_000, 11)

            Assertions.assertEquals(RankType.SILVER, rank.rankType)
        }

        @Test
        fun `Bronzeランクの会員を生成できる`() {
            val rank = Rank.create(4_000, 0)

            Assertions.assertEquals(RankType.BRONZE, rank.rankType)
        }