Featured image of post コード探偵ロックの事件簿【Specification】十二の証言〜散在するif文が隠した真犯人〜

コード探偵ロックの事件簿【Specification】十二の証言〜散在するif文が隠した真犯人〜

与信審査の条件が12箇所に散在しルール変更時に更新漏れが発生する問題を、Specificationパターンで条件をオブジェクト化し演算子オーバーロードで宣言的に合成するリファクタリング事例

雑居ビルの三階

Slackの匿名メッセージを、僕はもう三回読み返していた。

条件分岐の設計で詰まってるなら、雑居ビルの三階。変な看板の探偵事務所。腕は確か。

先月のインシデントから2週間。与信審査の信用スコア閾値を700から650に変更したとき、12箇所あるif文のうち3箇所を直し漏れた。本番環境で正規の顧客にローンが組めないと通報が入り、ホットフィックスを当てるまでの4時間、上長の表情は氷点下だった。

「全条件を一覧できるようにしろ。二度目はない」

その命令から2週間、僕は何も進んでいない。grepで条件を洗い出すことはできたが、12箇所の条件を「一覧できるように」する設計が思いつかなかった。共通関数に切り出すか。でもコンテキストが違うから引数が爆発する。定数にまとめるか。閾値だけまとめても条件の構造が散在している問題は解決しない。

だから今、僕はこのビルの階段を上っている。

三階の廊下の突き当たりに、すりガラスのドアがあった。「レガシー・コード・インベスティゲーション」と書かれた看板が、蛍光灯の明かりで妙に浮きあがっている。探偵事務所。コードの。本当にここでいいのだろうか。

ドアを開けた。

まず目に入ったのは、壁一面に貼られたコードのプリントアウトだった。A4用紙が20枚以上、画鋲で留められ、赤い糸で結ばれている。刑事ドラマの捜査ボードそのものだ。デスクの上にはモニターが3台、飲みかけのエナジードリンクの缶がいくつか散乱し、排熱で室温が妙に高い。

その奥に男がいた。コートを着たまま、赤い糸のひとつをつまんで眺めていた。

「12件か」

僕はまだ何も言っていない。

「条件分岐の散在で来たのだろう。12は勘だがね。与信系のレガシーコードでは、よくある数だ」

男が振り返った。鋭い目が、僕のノートPCのロゴを一瞥した。

「レガシー・コード・インベスティゲーション所長、ロック。——座りたまえ、ワトソン君」

「……僕には名前があるんですが」

ロックさんはすでにモニターに向き直っていた。

散在する指紋

「コードを見せたまえ」

促されるままノートPCを開き、与信審査システムのリポジトリを見せた。ロックさんは画面を覗き込み、ターミナルで何かを打ち始めた。

grep -rn 'credit_score' src/ と。——ほう、14件。そのうち条件判定に使われているのが12件。残り2件は代入だ」

「はい。信用スコアの閾値判定が12箇所に散らばっています。CreditAssessorReportGenerator、APIコントローラ、バッチ処理……全部微妙にコンテキストが違うので、共通関数にまとめきれなくて」

ロックさんが僕のコードの一つを指さした。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
package CreditAssessor;
use Moo;
use v5.36;

sub assess ($self, $customer) {
    unless ($customer->is_active) {
        return { approved => 0, reason => 'inactive account' };
    }
    unless ($customer->credit_score > 700) {
        return { approved => 0, reason => 'low credit score' };
    }
    if ($customer->has_overdue_payments) {
        return { approved => 0, reason => 'overdue payments exist' };
    }
    return { approved => 1, reason => 'all checks passed' };
}

「これが本丸の CreditAssessor だね。ではレポート生成側は」

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
package ReportGenerator;
use Moo;
use v5.36;

sub eligible_customers ($self, @customers) {
    return grep {
        $_->is_active
        && $_->credit_score > 700
        && !$_->has_overdue_payments
    } @customers;
}

「同じ条件が別の場所にも書かれている。そしてAPIコントローラ側はこうなっている、と」

1
2
3
4
5
6
7
# APIコントローラ — 先月の更新漏れ箇所
sub handle_application ($self, $customer) {
    # 閾値が700→650に変更されたのに、ここだけ古い値のまま
    if ($customer->credit_score > 700 && $customer->is_active) {
        # ...
    }
}

ロックさんが椅子の背にもたれた。

「12人の証言者が、それぞれ別の法廷で勝手に証言している。しかも——3人の証言内容が食い違っている。閾値が700のままの箇所が残っているわけだ」

「はい。先月のインシデントがまさにそれです」

「Shotgun Surgery。散弾銃手術の典型だね。弾は12発、命中したのは9発。残り3発は壁に当たって跳弾した。跳弾した弾が、顧客に当たった」

言い方は大げさだったが、事実関係は正確だった。

「grepで全箇所を見つけて直す、では答えになりませんよね。次にルールが変わったとき、また同じことが起きます」

「その通りだ、ワトソン君。証言者を一人ずつ追いかけるのは捜査員の仕事ではない。証言者を法廷に集めて、一枚の判決文にまとめるのが探偵の仕事だ」

「僕には名前が——」

「さて、本題に入ろう」

完全に無視された。……もういいです。

証言者に名前を与える

ロックさんが壁の捜査ボードから紙を一枚はがし、白い面を上にしてデスクに置いた。

「まず基本的な問いだ。$customer->credit_score > 700 という条件は、何者だね」

「……条件式です。if文の中の」

「それは形式だ。意味を聞いている。このコードが表現しているビジネスルールは何だ」

「『信用スコアが基準値を超えていること』……ですか」

「そうだ。ならば名前を付けたまえ。証拠品番号のまま法廷に出してはいけない。証言者の名前を呼ぶのだ」

ロックさんがペンを取り、紙に書き始めた。

GoodCreditSpec。これが証言者の名前だ。Specification パターン——条件をオブジェクトにする」

「条件をオブジェクトにする、ですか。if文をクラスでラップしただけじゃないですか。何が嬉しいんですか」

ロックさんの目が光った。待っていた質問だ、とでも言いたげに。

「名前が付くのだよ、ワトソン君。$credit_score > 700 はただの証拠品番号だ。12箇所に散在していても、grepでは同一のルールだとわからない。だが GoodCreditSpec証言者の名前だ。名前があれば、いつでも、どこからでも、法廷に呼び出せる」

ロックさんがコードを書き始めた。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
package Specification;
use Moo::Role;
use v5.36;
use overload
    '&' => \&and_spec,
    '|' => \&or_spec,
    '!' => \&not_spec,
    'bool' => sub { 1 },
    fallback => 1;

requires 'is_satisfied_by';

sub and_spec ($self, $other, @) {
    return Specification::And->new(left => $self, right => $other);
}

sub or_spec ($self, $other, @) {
    return Specification::Or->new(left => $self, right => $other);
}

sub not_spec ($self, @) {
    return Specification::Not->new(spec => $self);
}

Specification ロール。Moo::Roleで定義する。これを with したクラスは、is_satisfied_by メソッドを持つ証言者になる」

requires 'is_satisfied_by' ……これはインターフェースですか」

「そうだ。Javaの interface に相当する。このロールを持つ者は必ず is_satisfied_by で真偽を答えなければならない。証言台に立つ資格だ」

use overload の部分は……」

「後で見せる。まずは証言者を作ろう」

1
2
3
4
5
6
7
8
package Spec::ActiveCustomer;
use Moo;
use v5.36;
with 'Specification';

sub is_satisfied_by ($self, $customer) {
    return $customer->is_active;
}
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
package Spec::GoodCredit;
use Moo;
use v5.36;
with 'Specification';

has threshold => (is => 'ro', default => 650);

sub is_satisfied_by ($self, $customer) {
    return $customer->credit_score > $self->threshold;
}
1
2
3
4
5
6
7
8
package Spec::NoOverdue;
use Moo;
use v5.36;
with 'Specification';

sub is_satisfied_by ($self, $customer) {
    return !$customer->has_overdue_payments;
}

ActiveCustomerSpecGoodCreditSpecNoOverdueSpec。3人の証言者が、それぞれひとつの条件だけを担当する」

「ここで質問なんですが」

僕は手を挙げた。

「12箇所のif文を全部これで置き換えられることは、どうやって保証するんですか。名前を付けたからって、旧い条件式が自動で消えるわけじゃないですよね」

ロックさんが薄く笑った。

「良い疑いだ。12の証言者を法廷に立たせれば、一人ずつ is_satisfied_by で尋問できる。そして——」

画面に1行を書いた。

1
my @eligible = grep { $spec->is_satisfied_by($_) } @customers;

「旧いif文をこの1行に置き換えたとき、テストが通るかどうかが証明になる。grepは1行だが、その中で証言しているのは仕様オブジェクトだ。嘘をついている者——バグ——がいれば、テストが炙り出す」

判決文の読み上げ

「ここからが本番だ」

ロックさんが overload の話に戻った。

「3人の証言者は揃った。だが裁判では、個々の証言を組み合わせて判決を下す。AND、OR、NOT——合成だ」

まず合成クラスが画面に現れた。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
package Specification::And;
use Moo;
use v5.36;
with 'Specification';

has left  => (is => 'ro', required => 1);
has right => (is => 'ro', required => 1);

sub is_satisfied_by ($self, $candidate) {
    return $self->left->is_satisfied_by($candidate)
        && $self->right->is_satisfied_by($candidate);
}
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
package Specification::Or;
use Moo;
use v5.36;
with 'Specification';

has left  => (is => 'ro', required => 1);
has right => (is => 'ro', required => 1);

sub is_satisfied_by ($self, $candidate) {
    return $self->left->is_satisfied_by($candidate)
        || $self->right->is_satisfied_by($candidate);
}
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
package Specification::Not;
use Moo;
use v5.36;
with 'Specification';

has spec => (is => 'ro', required => 1);

sub is_satisfied_by ($self, $candidate) {
    return !$self->spec->is_satisfied_by($candidate);
}

Specification::And は2つの仕様を受け取り、両方が真のときだけ真を返す。OrNot も同様だ。これらも Specification ロールを持っているから、合成した結果をさらに合成できる」

「Composite パターンですね」

「その通り。そして先ほどの use overload だ。Perlの &|! 演算子をオーバーロードすることで、こう書ける——」

ロックさんが画面に一行を打った。そして椅子から立ち上がり、芝居がかった口調で読み上げた。

1
2
3
my $eligible = Spec::ActiveCustomer->new
    & Spec::GoodCredit->new(threshold => 650)
    & Spec::NoOverdue->new;

『被告は、活動中であり、信用良好であり、延滞していない場合に限り、与信を認める』——判決文の読み上げだ、ワトソン君」

正直に言えば、少し感心した。コードが自然言語のように読める。だが僕はもう少し掘りたかった。

「条件ごとにクラスを作るということは、12箇所分だと数十個のクラスが必要になりませんか。クラスの爆発が心配なんですが」

「12の証言者には12の名前が必要だ。だが全員が法廷に個別出廷するわけではない。合成がある。$active & $good_credit & !$overdue ——判決文は3人の証言から1行で書ける。12の条件のうち重複しているものは1つにまとまるから、実際のクラス数はもっと少ない」

「なるほど……でも &| って、ビット演算と紛らわしくないですか。コードレビューで混乱しそうです」

「では比較したまえ。判決文が ->and_spec()->and_spec()->or_spec()->not_spec() のメソッドチェーンになるのと、$a & $b | !$c になるのと。裁判長はどちらを読みたいかな」

メソッドチェーンの方が冗長なのは確かだった。演算子オーバーロードは use overload で定義されており、ビット演算とは文脈が違う。仕様オブジェクト同士の演算だとわかっていれば、混乱は起きないだろう。

「では、この仕様オブジェクトを使って CreditAssessor をリファクタリングする」

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
package CreditAssessor;
use Moo;
use v5.36;
use Types::Standard qw(ConsumerOf);

has spec => (
    is       => 'ro',
    isa      => ConsumerOf['Specification'],
    required => 1,
);

sub assess ($self, $customer) {
    if ($self->spec->is_satisfied_by($customer)) {
        return { approved => 1, reason => 'all checks passed' };
    }
    return { approved => 0, reason => 'credit check failed' };
}

CreditAssessor はもう条件を知らない。仕様オブジェクトに尋問を委ねるだけだ。閾値が変わっても、CreditAssessor 自体は一切修正しない。GoodCredit->new(threshold => 650) の部分を変えれば、12箇所すべてに反映される」

ReportGenerator も同じ仕様を受け取るようにすれば……」

「そうだ。同じ判決文を使う。Single Source of Truth。条件の定義は一箇所、それを参照する場所は何箇所あっても構わない」

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
package ReportGenerator;
use Moo;
use v5.36;
use Types::Standard qw(ConsumerOf);

has spec => (
    is       => 'ro',
    isa      => ConsumerOf['Specification'],
    required => 1,
);

sub eligible_customers ($self, @customers) {
    return grep { $self->spec->is_satisfied_by($_) } @customers;
}

「先月のインシデントでは、ApiController だけ閾値が700のまま残っていた。この設計なら——」

ApiController も同じ $eligible を受け取る。閾値は Spec::GoodCreditthreshold 属性に集約されているから、変更は一箇所。跳弾は起きない」

クラス構造を整理すると、こうなる。

	classDiagram
    class Specification {
        <<Role>>
        +is_satisfied_by(candidate) bool
        +and_spec(other) Specification::And
        +or_spec(other) Specification::Or
        +not_spec() Specification::Not
    }

    class Specification_And {
        +left: Specification
        +right: Specification
        +is_satisfied_by(candidate) bool
    }

    class Specification_Or {
        +left: Specification
        +right: Specification
        +is_satisfied_by(candidate) bool
    }

    class Specification_Not {
        +spec: Specification
        +is_satisfied_by(candidate) bool
    }

    class Spec_ActiveCustomer {
        +is_satisfied_by(customer) bool
    }

    class Spec_GoodCredit {
        +threshold: Int
        +is_satisfied_by(customer) bool
    }

    class Spec_NoOverdue {
        +is_satisfied_by(customer) bool
    }

    class CreditAssessor {
        +spec: Specification
        +assess(customer) HashRef
    }

    Specification <|.. Specification_And
    Specification <|.. Specification_Or
    Specification <|.. Specification_Not
    Specification <|.. Spec_ActiveCustomer
    Specification <|.. Spec_GoodCredit
    Specification <|.. Spec_NoOverdue

    CreditAssessor --> Specification

法廷の秩序

「では確認だ。テストを回そう」

ロックさんがテストコードを画面に映した。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
use Test::More;

# --- 各仕様クラスの独立テスト ---

subtest 'ActiveCustomerSpec' => sub {
    my $spec = Spec::ActiveCustomer->new;
    ok($spec->is_satisfied_by(MockCustomer->new(is_active => 1)),
        'アクティブ顧客は満たす');
    ok(!$spec->is_satisfied_by(MockCustomer->new(is_active => 0)),
        '非アクティブ顧客は満たさない');
};

subtest 'GoodCreditSpec' => sub {
    my $spec = Spec::GoodCredit->new(threshold => 650);
    ok($spec->is_satisfied_by(MockCustomer->new(credit_score => 700)),
        '700 > 650 で満たす');
    ok(!$spec->is_satisfied_by(MockCustomer->new(credit_score => 650)),
        '650 は 650 を超えないので満たさない');
};

# --- 演算子オーバーロードによる合成テスト ---

subtest '演算子合成' => sub {
    my $eligible = Spec::ActiveCustomer->new
        & Spec::GoodCredit->new(threshold => 650)
        & Spec::NoOverdue->new;

    ok($eligible->is_satisfied_by(MockCustomer->new(
        is_active => 1, credit_score => 700, has_overdue_payments => 0,
    )), '全条件クリア');

    ok(!$eligible->is_satisfied_by(MockCustomer->new(
        is_active => 1, credit_score => 600, has_overdue_payments => 0,
    )), 'スコア不足で不合格');
};

# --- 閾値のSingle Source of Truth ---

subtest '閾値が一箇所で管理される' => sub {
    my $eligible = Spec::ActiveCustomer->new
        & Spec::GoodCredit->new(threshold => 650)
        & Spec::NoOverdue->new;

    my $assessor  = CreditAssessor->new(spec => $eligible);
    my $generator = ReportGenerator->new(spec => $eligible);

    my $customer = MockCustomer->new(
        credit_score => 680, is_active => 1, has_overdue_payments => 0,
    );

    is($assessor->assess($customer)->{approved}, 1,
        'CreditAssessor: 680 > 650 で承認');

    my @list = $generator->eligible_customers($customer);
    is(scalar(@list), 1,
        'ReportGenerator: 同じ仕様で同じ結果');
};

done_testing;

テストを実行する。画面が一瞬スクロールして——全てグリーン。

「12の証言者が法廷に揃い、判決文が一枚にまとまった。閾値の変更は GoodCredit->new(threshold => ...) の1箇所を変えれば、全てのAssessorとGeneratorに反映される」

僕は画面を見つめていた。なるほど、とは思う。だが理屈でわかることと、自分の手で書けることは別だ。

「さて——」

ロックさんが突然、キーボードから手を離し、椅子を引いた。

「腕試しだ、ワトソン君。 『VIP顧客かつ年間購入額100万円以上、ただし反社チェック未完了は除外』 の判決文を書いてみたまえ」

「……僕がですか」

「証言者の作り方は見た。合成の仕方も見た。あとは書くだけだ」

ロックさんが腕を組んで壁にもたれた。

僕はキーボードに手を置いた。まず証言者を定義する。VIP顧客。

1
2
3
4
5
6
7
8
package Spec::VipCustomer;
use Moo;
use v5.36;
with 'Specification';

sub is_satisfied_by ($self, $customer) {
    return $customer->is_vip;
}

次に、年間購入額。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
package Spec::HighPurchase;
use Moo;
use v5.36;
with 'Specification';

has threshold => (is => 'ro', default => 1_000_000);

sub is_satisfied_by ($self, $customer) {
    return $customer->annual_purchase >= $self->threshold;
}

そしてコンプライアンスチェック。

1
2
3
4
5
6
7
8
package Spec::ComplianceChecked;
use Moo;
use v5.36;
with 'Specification';

sub is_satisfied_by ($self, $customer) {
    return $customer->compliance_checked;
}

合成する。

1
2
3
my $vip_eligible = Spec::VipCustomer->new
    & Spec::HighPurchase->new(threshold => 1_000_000)
    & Spec::ComplianceChecked->new;

テストを書く。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
subtest 'VIP適格判定' => sub {
    ok($vip_eligible->is_satisfied_by(MockCustomer->new(
        is_vip => 1, annual_purchase => 1_500_000, compliance_checked => 1,
    )), '全条件クリアのVIP');

    ok(!$vip_eligible->is_satisfied_by(MockCustomer->new(
        is_vip => 1, annual_purchase => 500_000, compliance_checked => 1,
    )), 'VIPだが購入額不足');

    ok(!$vip_eligible->is_satisfied_by(MockCustomer->new(
        is_vip => 1, annual_purchase => 1_500_000, compliance_checked => 0,
    )), 'VIPだがコンプラ未完了');
};

テストを実行した。グリーン。

ロックさんが壁から背を離した。画面を一瞥して、短く言った。

「……悪くないね」

それだけだった。それだけで十分だった。

階段を降りながら

帰り支度をしながら、ふと訊いた。

「この Specification パターンは、GoFの23パターンには入っていないんですか」

「入っていない。Eric EvansのDDD——ドメイン駆動設計の文脈で体系化されたパターンだ。だが構造としてはCompositeとStrategyの合成だ。GoFの血は流れている」

「なるほど」

「ただし万能ではない。証言者にする価値があるのは、ビジネスルールとして名前を持つべき条件だけだ。x > 0 のような自明なガードをいちいちクラスにする必要はない。判決文に書くべき証言と、廊下の立ち話を区別することだ」

事務所のドアを閉め、薄暗い階段を降りた。

Slackに匿名で書いてくれた誰かのことを思い出した。「腕は確か」。確かに、そうだった。

12箇所に散らばっていた条件は、今は名前を持つ仕様オブジェクトになった。閾値が変わっても、変更は一箇所。判決文は一枚。

階段の踊り場で、僕は小さく呟いた。

「判決文か……」

いつか僕も、Slackで誰かを案内できるかもしれない。


探偵の調査報告書

容疑(アンチパターン)真実(パターン)証拠(効果)
散在するif文(Shotgun Surgery)Specification パターン(条件のオブジェクト化)ルール変更が1箇所で完結する
ルールの重複・矛盾(DRY違反)Single Source of Truth(仕様クラスに集約)更新漏れによるインシデントが防止される
条件の複雑化(Conditional Complexity)AND/OR/NOT合成による宣言的記述コードがビジネスルールとして読める
テスト困難性(条件式の分離不可)仕様クラスの独立ユニットテスト各条件を個別に検証できる

推理のステップ

  1. 証言者の洗い出し: grep でコードベース全体の条件判定を検索し、散在するビジネスルールと重複を特定する
  2. 名前を与える: 各条件に仕様クラス名を付ける。Spec::ActiveCustomerSpec::GoodCredit など、ドメインの語彙で命名する
  3. Specification ロールの定義: Moo::Rolerequires 'is_satisfied_by' のロールを作り、and_spec/or_spec/not_spec メソッドを提供する
  4. 演算子オーバーロード: use overload&|! をオーバーロードし、宣言的な合成構文を実現する
  5. 合成クラスの実装: Specification::AndSpecification::OrSpecification::Not の3つの合成クラスを作る
  6. 利用側の書き換え: 旧いif文を $spec->is_satisfied_by($customer) に置き換え、仕様オブジェクトは外部からDIで渡す
  7. テスト: 各仕様クラスを独立してテストし、合成結果もテストする

ロックより

条件が散在しているとき、真犯人はif文ではない。if文に名前を与えなかった我々自身だ。

名前のない証言者は法廷に立てない。名前のない条件は管理できない。12箇所に散らばったcredit_score > 700は、12の匿名の証言だ。匿名の証言は食い違い、食い違いはインシデントを招く。

名前を与え、法廷に集め、一枚の判決文にまとめたまえ。そうすれば二度と跳弾は起きない。

comments powered by Disqus
Hugo で構築されています。
テーマ StackJimmy によって設計されています。