ディシジョンテーブルをカルノー図で簡単化するか？

f:id:Naotsugu:20160715001624p:plain

SlideShare でこんなスライドがあった。

エスイーが要件定義でやるべきたったひとつのこと from Yoshitka Kawashima

www.slideshare.net

ディシジョンテーブルはいいけど、カルノー図かいて整理してロジックに落としましょう。という至極まっとうなもの。

内容は、

ユーザプロフィールの登録状況に応じて連絡手段を決める際、以下のディシジョンテーブルがあったとすると。

電話番号	メールアドレス	住所	連絡手段
○	○	○	メール
○	○	☓	メール
○	☓	○	DM
○	☓	☓	電話
○	○	○	メール
○	○	☓	メール
☓	☓	○	DM
☓	☓	☓	なし

これをそのままコードに落とすと、サイクロマティック複雑度が増します。

f:id:Naotsugu:20160715000000p:plain:w200

ではなくて、以下のように整理すると、

電話:メール	00	01	11	10
住所 : 0	なし	MAIL	MAIL	TEL
住所 : 1	DM	MAIL	MAIL	DM

つまりは、

メールアドレスが登録されていればメールで
メールアドレスが未登録だが、住所が登録されていればDMで
メールアドレスも住所も未登録だが、電話番号が登録されていれば電話で

と整理して、整理した結果からコードに落としましょうと。

if (StringUtils.isNotEmpty(email)) {
    contact = MAIL;
} else {
    if (StringUtils.isNotEmpty(address)) {
        contact = DM;
    } else {
        contact = StringUtils.isNotEmpty(tel) ? TEL : NO;
    }
}

でも

書くならこの方が良いですよね。

if (StringUtils.isNotEmpty(email)) {
    contact = MAIL;
} else if (StringUtils.isNotEmpty(address)) {
    contact = DM;
} else if (StringUtils.isNotEmpty(tel)) {
    contact = TEL;
} else {
    contact = NO;
}

ある条件によって、contact に必ずいずれかの設定を行う、という意図が読み取りやすいので。

または、三項演算子も書き方によっては読みやすいし、シンプルだし、

contact = StringUtils.isNotEmpty(email)   ? MAIL 
        : StringUtils.isNotEmpty(address) ? DM
        : StringUtils.isNotEmpty(tel)     ? TEL
        : NO;

でも、

要件はたえず変わり得るもので、

全ての連絡手段が登録済みの顧客は、重要顧客として即時性のあるTELによる連絡にしたい。

とかいう話がでたら、

電話:メール	00	01	11	10
住所 : 0	なし	MAIL	MAIL	TEL
住所 : 1	DM	MAIL	TEL	DM

ロジックに落としにくいですね。

なので、

サイクロマティック複雑度上げないような形でディシジョンテーブルをそのまま表現しても良いのではないでしょうかね。

public static Map<List<Boolean>, Contact> table = new HashMap() {{
    // user profile     (tel,   mail,   address)
    put(ImmutableList.of(true,  true,   true ), MAIL);
    put(ImmutableList.of(true,  true,   false), MAIL);
    put(ImmutableList.of(true,  false,  true ), DM  );
    put(ImmutableList.of(true,  false,  false), TEL );
    put(ImmutableList.of(false, true,   true ), MAIL);
    put(ImmutableList.of(false, true,   false), MAIL);
    put(ImmutableList.of(false, false,  true ), DM  );
    put(ImmutableList.of(false, false,  false), NO );
}};

単体テスト書くまでもなく視覚的に正しさに自身が持てますし。

contact = table.get(ImmutableList.of(
        isNotBlank(tel),
        isNotBlank(email),
        isNotBlank(address)));

も少しやると、

こんな感じの用意しておけば、

public class DecisionTable<C, A> {

    private final Map<List<C>, A> map;

    public DecisionTable() {
        map = new HashMap<>();
    }

    public Rule rule(C...conditions) {
        return new Rule(this, conditions);
    }

    public A get(C...conditions) {
        return map.get(Arrays.asList(conditions));
    }

    DecisionTable bind(List<C> conditions, A action) {
        if (map.containsKey(conditions)) 
                throw new IllegalArgumentException();
        map.put(conditions, action);
        return this;
    }

    class Rule<C, A> {
        private final DecisionTable dt;
        private final List<C> conditions;

        Rule(DecisionTable dt, C...conditions) {
            this.dt = dt;
            this.conditions = Arrays.asList(conditions);
        }
        public DecisionTable action(A action) {
            return dt.bind(conditions, action);
        }
    }
}

少し見やすくなるかな。

DecisionTable<Boolean, Contact> decisionTable = new DecisionTable<>()
    //   (tel,   mail, address)
    .rule(true,  true,   true ).action(MAIL)
    .rule(true,  true,   false).action(MAIL)
    .rule(true,  false,  true ).action(DM  )
    .rule(true,  false,  false).action(TEL )
    .rule(false, true,   true ).action(MAIL)
    .rule(false, true,   false).action(MAIL)
    .rule(false, false,  true ).action(DM  )
    .rule(false, false,  false).action(NO );

decisionTable.get(
    isNotBlank(tel),
    isNotBlank(email),
    isNotBlank(address));