Featured image of post コード探偵ロックの事件簿【Refused Bequest】相続放棄できない遺産〜壊れた家系図と die で殺されたメソッド〜

コード探偵ロックの事件簿【Refused Bequest】相続放棄できない遺産〜壊れた家系図と die で殺されたメソッド〜

親クラスの機能を空実装やdieで潰すRefused Bequest——is-a関係が成り立たない不適切な継承をComposition over Inheritanceで解消するコード探偵ロックの推理。extends除去・Role分離・handles委譲の3段リファクタリング。

先月の飲み会で森川さんから聞いた話を、私はずっと気にしていた。

「コードの設計を見てくれる変わった人がいるんですよ」

森川さんとは別のプロジェクトだが、同じ社内勉強会には何度か顔を合わせている。彼は EC システムの配送料計算を壊したときに助けてもらったらしい。「レガシー・コード・インベスティゲーション」という怪しい名前の事務所を構えていて、コードの悪臭を嗅ぎ分けるのだとか。

正直、半信半疑だった。

だが、先週の出来事で気持ちが変わった。新人の八木が、私が3年前に設計した基底クラス BaseDocument のコードを読んで、こう聞いてきたのだ。

「柴田さん、CreditNoteBaseDocument を継承しているのに、なぜ validatedie するんですか?使えないメソッドがあるなら、なぜ継承しているんですか?」

答えられなかった。

その夜、検索で見つけた LCI のウェブサイトから問い合わせフォームを送った。翌日の返信は短かった。「現場を見なければ推理はできない。伺う」。

会議室Bの来客

3日後、会議室Bにその男が現れた。

ロックと名乗った。年齢不詳。スーツの上に薄手のコートを羽織り、まるで現場検証に来た刑事のような格好をしている。会議室に入るなり、ホワイトボードのマーカーを全色引き出して色味を確認し始めた。

「赤が薄い」

「はい?」

「推理に赤は不可欠だ。家系図の断絶を示すには赤がなければならない。……まあいい、青で代用しよう」

森川さんが「変な人」としか言わなかった理由がわかった。変なのは確かだが、それ以上に芝居がかっている。探偵を気取っているのか、探偵そのものなのか、判断がつかない。

壁に貼ってあったプロジェクトの組織図を一瞥して、「これは人間の組織図か。コードの組織図はどこだ」と言った。

私はプロジェクターを起動し、クラス階層を映した。

「3年前に私が設計した基底クラスです。BaseDocument という名前で、帳票の共通処理をまとめています。InvoiceReceipt は問題なく使っていますが、CreditNote だけが——」

ロックさんは映された画面に近づいた。CreditNote のメソッド一覧に目を走らせ、一箇所で止まった。

sub validate ($self) { die "CreditNote does not use BaseDocument validation" }

「……柴田さん。このメソッドは何をしている」

「何も。親クラスの validate は正の金額を要求しますが、CreditNote は返金なので負の金額を扱います。契約が合わないので——」

「殺したのだな」

静かな声だった。

die で殺されたメソッド

ロックさんはホワイトボードにクラス図を描き始めた。BaseDocument を頂点に、InvoiceCreditNote が矢印でぶら下がっている。

BaseDocument は4つのメソッドを持っている。validateprint_headerprint_footerformatInvoice はこの4つをすべて使っている。正当な相続人だ」

青いマーカーが CreditNote の横に回った。

「ところが CreditNote はどうだ。validatedie で殺した。print_headerprint_footer は空文字列を返して黙殺した。format は親の実装とは全く異なる形に上書きした」

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
package CreditNote;
use v5.36;
use Moo;

extends 'BaseDocument';

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

# 🚫 親の validate は正の金額を要求するが、CreditNote は負の金額
sub validate ($self) {
    die "CreditNote does not use BaseDocument validation";
}

# 🚫 CreditNote はデジタル専用。印刷機能は不要
sub print_header ($self) { return "" }
sub print_footer ($self) { return "" }

# 親の format を完全に上書き
sub format ($self) {
    return sprintf "CREDIT %s / 返金: ¥%s / 理由: %s / 元請求: %s",
        $self->number, abs($self->amount), $self->reason,
        $self->original_invoice_number;
}

「4つのメソッドのうち4つを拒否している。相続財産の全額を拒んでいるのに、相続人を名乗っている」

「当時は帳票という共通点で括るのが妥当だと判断しました。numberdateamountcustomer_name——属性は共通ですし」

「属性が同じだから親子?」

ロックさんはホワイトボードの隅に小さな絵を描いた。犬と椅子。両方に「脚: 4本」と書き添えた。

「犬と椅子は脚が4本ある。だが椅子は犬の子供ではない」

反論しようとして、やめた。属性の一致は、is-a の証明にはならない。それは知っていたはずだ。3年前の私が見て見ぬふりをしただけだ。

「もう一つ見せたい証拠がある」

ロックさんは DocumentProcessor のコードを開いた。

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

sub process_all ($self, @docs) {
    my @results;
    for my $doc (@docs) {
        $doc->validate;
        push @results, $doc->format;
    }
    return @results;
}

「このメソッドは BaseDocument を信頼している。引数のオブジェクトが validate を呼べば検証を通過し、format を呼べばフォーマットされた文字列が返ると。Invoice を渡せば問題ない。では CreditNote を渡したらどうなる」

わかっていた。validatedie する。

BaseDocument を期待するコードに CreditNote を渡すと、処理が破綻する。代わりが務まらない。これがリスコフの置換原則の違反だ——親のインスタンスを子で差し替えても振る舞いが変わらないという契約を、CreditNote は3箇所で破っている」

	classDiagram
    class BaseDocument {
        +number
        +date
        +amount
        +customer_name
        +validate()
        +print_header()
        +print_footer()
        +format()
    }
    class Invoice {
        +due_date
    }
    class CreditNote {
        +reason
        +original_invoice_number
        +validate() ⚠️ die
        +print_header() ⚠️ 空
        +print_footer() ⚠️ 空
        +format() ⚠️ 完全上書き
    }

    BaseDocument <|-- Invoice : extends ✓
    BaseDocument <|-- CreditNote : extends ✗ Refused Bequest

ロックさんはホワイトボードの CreditNote の横に大きく書いた。

Refused Bequest ——拒否された遺産

「この悪臭の名前だ。親から受け取った遺産を、子が拒否している。使わないメソッドを空にし、合わない契約を die で殺す。相続人を名乗りながら、相続財産を裏口から捨てている」

しばらく黙った。八木の質問が頭の中で繰り返されていた。「なぜ継承しているのに使えないメソッドがあるんですか」。答えられなかったのは、答えがなかったからではなく、答えを認めたくなかったからかもしれない。

「……では、どう直せばいいんですか」

家系図を引き直す

ロックさんはホワイトボードを消し、新しい図を描き始めた。

「3段階で進める。第一に、共通の契約を抽出する。第二に、能力を分離する。第三に、関係を正す」

Step 1: 共通の契約を Role に抽出する

「まず BaseDocument の本質を考えたまえ。InvoiceCreditNote に共通する契約は何だ」

formatvalidate を持つこと。属性としては numberdate

「そのとおり。その契約を、基底クラスではなく Role として定義する」

1
2
3
4
5
6
7
8
package Documentable;
use Moo::Role;
use v5.36;

requires 'format', 'validate';

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

requires は契約の宣言だ。『このRoleを合成するクラスは、formatvalidate を自分で実装しなければならない』と明示している。基底クラスの validate のように中身を押しつけるのではなく、メソッドの存在だけを保証する」

「Roleにすると、validate の中身は各クラスに書くことになりますよね。コードの重複が増えませんか」

ロックさんは首を横に振った。

「重複しているのは見た目が同じコードだ。しかし Invoicevalidate は『正の金額であること』を保証し、CreditNotevalidate は『負の金額であること』を保証する。同じ名前で違う契約だ。無理に一つにまとめることが、今回の事件の原因だったはずだ」

そのとおりだ。BaseDocumentvalidate は「正の金額」を前提としていた。その前提が CreditNote に合わないから、die で殺すしかなかった。

Step 2: 印刷機能を別の Role に分離する

「次に print_headerprint_footer だ。Invoice には必要だが、CreditNote には不要だった。ならばこの能力を別の Role として切り出す」

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
package Printable;
use Moo::Role;
use v5.36;

requires 'amount', 'customer_name';

sub print_header ($self) {
    return sprintf "=== %s No.%s ===\n日付: %s",
        ref($self), $self->number, $self->date;
}

sub print_footer ($self) {
    return sprintf "--- 合計: ¥%s ---", $self->amount;
}

InvoiceDocumentablePrintable の両方を合成する。CreditNoteDocumentable だけを合成する。必要な能力だけを選べる」

extends で一括で受け取っていたものを、with で個別に選ぶということですか」

「そのとおりだ。継承は遺産相続だ——要るものも要らないものも全部受け取る。Role は能力の選択だ——必要な技能だけを身につける。相続放棄はできないが、どの技能を学ぶかは選べる」

リファクタリング後の Invoice はこうなる。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
package Invoice;
use v5.36;
use Moo;

has amount        => (is => 'ro', required => 1);
has customer_name => (is => 'ro', required => 1);
has due_date      => (is => 'ro', required => 1);

with 'Documentable', 'Printable';

sub validate ($self) {
    die "amount must be positive"   unless $self->amount > 0;
    die "customer_name is required" unless length $self->customer_name;
    return 1;
}

sub format ($self) {
    return join "\n",
        $self->print_header,
        sprintf("顧客: %s  金額: ¥%s", $self->customer_name, $self->amount),
        $self->print_footer;
}

「一点、実装上の注意がある」

ロックさんがコードの haswith の順序を指さした。

「Moo の requires は、Role を合成する時点でそのメソッドが存在するかどうかを検証する。Printableamountcustomer_name を要求している。has によるアクセサ宣言が with よりになければ、合成時に『メソッドが見つからない』とエラーになる。has を先、with を後」

これは知らなかった。use Moo の直後に with を書く癖があったが、requires のある Role を合成するときは順序が重要なのか。

Step 3: has + handles による委譲

「もう一つ」

ロックさんがホワイトボードに矢印を描いた。CreditNote から Invoice へ。

CreditNotecustomer_name はどこから来ている」

BaseDocument の属性として……いえ、今は BaseDocument を外したから——」

「元の請求書だ。返金伝票は、必ず元の請求書に紐づいている。customer_name は元の請求書が持っている。同じ情報を二箇所に持つのは証拠のコピーだ。原本は元の請求書にある」

「コンストラクタで customer_name を渡す代わりに、元の請求書オブジェクトを渡して、そこから取得すると」

has で元の請求書を保持し、handles でメソッドを委譲する」

 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
package CreditNote;
use v5.36;
use Moo;

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

# has + handles: 元の請求書から顧客名と番号を委譲
has original => (
    is       => 'ro',
    required => 1,
    handles  => {
        customer_name   => 'customer_name',
        original_number => 'number',
    },
);

with 'Documentable';

sub validate ($self) {
    die "amount must be negative" unless $self->amount < 0;
    return 1;
}

sub format ($self) {
    return sprintf "CREDIT %s / 返金: ¥%s / 顧客: %s / 理由: %s / 元請求: %s",
        $self->number, abs($self->amount), $self->customer_name,
        $self->reason, $self->original_number;
}

$credit_note->customer_name を呼ぶと、内部で $credit_note->original->customer_name に委譲される。original_number は元の請求書の number に名前を変えて委譲している」

「委譲で元の請求書から取るのは、依存が変わっただけではないですか。前は BaseDocument に依存し、今は Invoice に依存している」

ロックさんは指を一本立てた。

「以前は血統に依存していた——BaseDocument の子供であるということに。今は関係に依存している——元の請求書を持っているということに。血統は選べないが、関係は選べる」

もう一歩踏み込みたかった。「実務上の違いは何ですか」

「テストだ。original にモックを渡せば CreditNote を単独でテストできる。extends では親クラスのすべての振る舞いが常に付いて回る。切り離せない。has なら、差し替えられる」

家系図のない世界

リファクタリング後のクラス図をロックさんがホワイトボードに描いた。

	classDiagram
    class Documentable {
        <<Role>>
        +number
        +date
        +validate()*
        +format()*
    }
    class Printable {
        <<Role>>
        +print_header()
        +print_footer()
    }
    class Invoice {
        +amount
        +customer_name
        +due_date
        +validate()
        +format()
    }
    class CreditNote {
        +amount
        +reason
        +original
        +validate()
        +format()
    }

    Documentable <|.. Invoice : with
    Printable <|.. Invoice : with
    Documentable <|.. CreditNote : with
    Invoice <-- CreditNote : has + handles

「矢印の向きが変わっている」

「以前は BaseDocument から下に向かって継承の矢印が伸びていた。今は DocumentablePrintable に向かって合成の点線が伸びている。血統ではなく能力で結ばれている」

CreditNotePrintable を持っていないことが、図を見ただけでわかりますね」

「それが設計の可読性だ。extends では『全部もらって要らないものを捨てる』だったが、with では『必要なものだけ拾う』になった」

最後に DocumentProcessor を書き換えた。

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

sub process_all ($self, @docs) {
    my @results;
    for my $doc (@docs) {
        $doc->validate;
        push @results, $doc->format;

        if ($doc->does('Printable')) {
            push @results, $doc->print_header;
        }
    }
    return @results;
}

isadoes に変わっている」

isa は血統の確認だ——『あなたは BaseDocument の血を引いていますか』。does は能力の確認だ——『あなたは印刷できますか』。身分証明書ではなく、技能証明書を見るようになった」

InvoiceCreditNote を混ぜて process_all に渡しても——」

Invoicevalidate を通過し、format を返し、does('Printable') が真なので print_header も返す。CreditNote は自身の validate を通過し、自身の format を返し、does('Printable') が偽なので print_header は呼ばれない。誰も die しない」

テストの結果が頭の中で組み上がった。CreditNote はもう BaseDocument の代わりを務める必要がない。自分の契約だけを守ればいい。

「3年前の私は、血統を揃えることが設計だと思っていたんですね」

ロックさんは答えなかった。代わりにスマートフォンを取り出して、ホワイトボードの図に向けた。

「この図は証拠品だ。接収する」

「それは会社の備品です。写真だけにしてください」

渋々撮影してスマートフォンをしまったロックさんは、マーカーのキャップを閉めながらこう言った。

「継承は契約だ、柴田さん」

一瞬間を置いて、私を見た。

「いや——ワトソン君」

不思議なもので、不快ではなかった。むしろその呼び方に切り替わったことが、何かを認めてもらったような気がした。自分の設計の間違いを自分で口にしたから、かもしれない。

「契約を守れないなら、契約の形を変えればいい。extends は親の全財産を相続する契約だ。全財産が要らないなら、必要な能力だけを選ぶ with に書き換えたまえ」

私は席を立った。

「私が設計した階層です。私が直します」


探偵の調査報告書

容疑(アンチパターン)真実(パターン)証拠(効果)
Refused Bequest(空メソッド・die で親を殺す)Composition over Inheritance不要な機能を継承しない
is-a 偽装(BaseDocumentCreditNotewith(Role)による能力の選択的合成契約違反が構造的に不可能になる
顧客情報の重複保持has + handles(委譲)データの原本は一箇所に

推理のステップ

  1. Refused Bequest を見つける: 子クラスのメソッドに空実装・die・未使用の override がないか探す。親から受け取ったメソッドを3つ以上拒否していたら Refused Bequest の疑い
  2. is-a 関係を問い直す: 「CreditNoteBaseDocument の一種か」をリスコフの置換原則で検証する。親を期待するコードに子を渡して die するなら、is-a は偽装
  3. 共通の契約を Role に抽出する: 基底クラスから requires で契約だけを定義する Role を切り出し、実装の押しつけを排除する
  4. 能力を別の Role に分離する: 一部のクラスにしか必要でない機能(印刷など)を独立した Role にし、with で選択的に合成する
  5. has + handles で関係を正す: 重複データを持つ代わりに、元のオブジェクトを has で保持し handles で委譲する。血統から関係への転換

ロックより

遺産を放棄したいなら、相続人をやめればいい。extends は親の全財産を受け取る契約だ。全財産が要らないのに相続人を名乗り、裏口から財産を捨てるのは設計の詐欺だ。with は能力の選択だ——必要な技能だけを身につけ、不要な契約に縛られない。血統で縛られるよりも、能力で結ぶ方が、コードはずっと自由になる。 | isa による血統チェック | does による能力チェック | 型ではなく契約で判断 |

推理のステップ

  1. extends で使っていないメソッド(空実装、die)を探す → Refused Bequest の検出
  2. 共通の契約requires)を Role に抽出する → Documentable
  3. 一部のクラスだけが必要な機能は別の Role にする → Printable
  4. データの重複は has + handles で委譲に置き換える
  5. isadoes に変えて、血統ではなく能力で判断する

ロックより

継承とは親子の絆ではない。契約だ。「私はあなたのすべてのメソッドを引き受け、あなたの代わりを務めます」という署名入りの約束だ。

その約束を守れないのであれば、そもそも署名してはならなかった。だが署名してしまったものは仕方がない。契約を破棄し、必要な能力だけを with で選び直したまえ。血統で縛られるよりも、能力で結ばれる方が、コードはずっと自由になる。

——そして、自分が書いた設計を自分で正す覚悟を持つ者は、ワトソン君と呼ばれるに値する。

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