Featured image of post コード探偵ロックの事件簿【Pipeline】五百行の迷路〜神メソッドを流れに変えた日〜

コード探偵ロックの事件簿【Pipeline】五百行の迷路〜神メソッドを流れに変えた日〜

500行の神メソッドに全処理が詰まり、1行直すと無関係な箇所が壊れる——その構造的原因をPipeline Patternで解消する。Perl/MooでStage Roleを定義し、各ステージを独立オブジェクト化、テスト可能な設計への変換を実装付きで解説。

事務所への来客 — 五百行の迷路

Googleマップが指し示す場所には、テナント看板の文字が色褪せて読めない雑居ビルが建っていた。

僕はスマホの画面とビルの外壁を交互に見比べた。間違いない。住所はここだ。でも、3階に何があるのか外からはわからない。意を決して1階の不動産屋に入り、カウンターに座っているおじさんに尋ねた。

「あの、3階に——レガシーなんとかっていう……」

おじさんが目を細めた。「ああ、あの探偵さんね。今日もたぶんいるよ」

探偵?

僕は礼を言って薄暗い階段を上った。2階を過ぎ、3階に着くと、ドアに手書きの紙が貼ってあった。

「LCI — Legacy Code Investigation」

問い合わせフォームの返信には住所しか書かれていなかった。サンプルコードを持参のこと、と一行だけ。ノックすると「開いている」と声がする。

ドアを開けた瞬間、チョークの粉の匂いがした。

壁一面に、配管の模式図が描かれていた。入力と書かれた箱から線が伸び、四角いフィルターを通過し、次のフィルターに繋がり、最後に出力の箱へ到達する。水道管の設計図のような図が、黒板代わりの壁を埋め尽くしていた。

図を描いている男は、僕に背を向けたままチョークを走らせていた。机の上には銅色のL字型の部品と、白い塩ビのT字型の部品が転がっている。マグカップから湯気が立ち上っている。

「……あの、すみません。ここ、配管業者さんじゃないですよね?」

男がチョークの手を止めた。振り返らないまま答える。

「ソフトウェアの配管はこっちだよ、ワトソン君」

僕はまだ名前を名乗っていなかった。

「え、あの……僕の名前は……」

ロックさんはすでに振り返っていた。ディスプレイの並ぶ机から椅子を引き寄せ、僕のほうを向いている。鋭い目だった。けれど、声のトーンは思ったほど冷たくなかった。

「サンプルコードは持ってきたかね?」

訂正のタイミングを完全に逃した。

この人が……不動産屋のおじさんが「探偵」と呼んでいたのは、こういうことか。確かに、普通の人では……ない。

僕はおそるおそるラップトップを開いた。

「これが……問題のコードです。process_order っていう関数で、受注の処理を全部やっています」

スクロールバーが小さい。500行以上ある。

ロックさんが画面を覗き込んだ。指でトラックパッドをスクロールしていく。長い沈黙。僕が不安になるくらい長い沈黙だった。

「何行目から何行目が割引の計算かね?」

「えっと…… 180行目あたりから……230行目くらいまで、だと思います」

「思います、か」

間があった。

「ワトソン君、この患者は重症だ」

ロックさんが壁の配管図を指差した。

「五百行の巨大な一室に、バリデーションと計算と整形と通知が同居している。しかも壁がない。仕切りがない。境界がない。割引の計算を直したら通知メールが壊れた、と問い合わせフォームに書いていたね?」

驚いた。確かに書いた。でも、すらすらと言い当てられるとは思わなかった。

「え……はい。なんで——」

「知っている必要はない。壁のない部屋で家具を動かせば、隣の住人にぶつかる。それだけのことだ」

コードの指紋 — 凶器は密結合

ロックさんが僕のラップトップを引き寄せた。すでに画面は process_order の先頭にスクロールされている。

 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
sub process_order ($order_data) {
    # -- バリデーション(ここから50行の検証ロジック) --
    die 'order_id is required' unless defined $order_data->{order_id};
    die 'items must be an array' unless ref $order_data->{items} eq 'ARRAY';
    # ... (中略) ...

    # -- 正規化(ここから30行) --
    for my $item ($order_data->{items}->@*) {
        $item->{name} =~ s/\A\s+|\s+\z//g;
        $item->{price} = int($item->{price});
    }

    # -- 計算(ここから80行) --
    my $subtotal = 0;
    for my $item ($order_data->{items}->@*) {
        $subtotal += $item->{price} * $item->{quantity};
    }
    my $tax   = int($subtotal * $order_data->{tax_rate});
    my $total = $subtotal + $tax;

    # -- 整形(ここから60行) --
    my @lines;
    for my $item ($order_data->{items}->@*) {
        push @lines, sprintf '%s x%d  ¥%d',
            $item->{name}, $item->{quantity},
            $item->{price} * $item->{quantity};
    }
    my $formatted = join "\n", @lines,
        "小計: ¥$subtotal", "税額: ¥$tax", "合計: ¥$total";

    # -- 通知(ここから40行) --
    my $body = "ご注文ありがとうございます。\n\n$formatted";

    return {
        %$order_data,
        subtotal => $subtotal, tax => $tax, total => $total,
        formatted => $formatted, notification_body => $body,
        notified => 1,
    };
}

ロックさんがコードの中に5つのコメントを書き込んだ。正確には、すでに僕が書いたコメントを指でなぞった。

「バリデーション。正規化。計算。整形。通知。——五つの処理ステップだ。それぞれが独立した仕事をしている……はずだ」

「はず?」

「だが現実には、バリデーションで作ったローカル変数を計算ステップが直接参照し、計算ステップが作った中間結果を整形ステップが暗黙に前提としている」

ロックさんが壁の配管図に視線を向けた。

「五人の職人が一つの作業台を共有している。誰かが工具を動かすと全員の手が止まる。——だから、割引を直したら通知が壊れたのだよ、ワトソン君」

……言われてみれば、確かにそうだ。$formatted$subtotal に依存していて、$subtotal$item->{price} に依存していて——全部、この一つのメソッドの中で、名前のわかりにくいローカル変数が糸のように絡み合っている。

「処理ステップの間に、壁がない。——これが『神メソッド』だよ、ワトソン君。すべてを知り、すべてを制御し、そして誰も手を入れられなくなった独裁者だ」

ロックさんが机上の銅色のエルボ継手を手に取って、僕に見せた。

「このエルボ継手は、パイプの向きを90度変える。だが口径は入口も出口も同じだ。何が通ろうが、入口の口径と出口の口径が合えば、どのパイプにでも繋がる

「はあ……」

「ステージも同じだ。入力の形と出力の形を揃えれば、自由に組み替えられる。——神メソッドを流れに変えよう」

鮮やかなリファクタリング — パイプラインの設計図

ロックさんが壁に新しい図を描き始めた。今度はクラス図のように見える。僕はラップトップの横にメモ帳を出した。

Stage Role — パイプの規格

「まず、全ステージが守るべき契約を定義する」

1
2
3
package Role::Stage;
use Moo::Role;
requires 'process';

「requires って、何を……強制しているんですか?」

requires 'process' は契約だ。この Role を with で取り込むクラスは、process メソッドを必ず実装しなければならない。実装し忘れたらどうなるか?——Perl がコンパイル時にエラーを出す。パイプラインに入る資格のないステージを門前払いするのが Role の仕事だ」

メモ帳にペンを走らせながら、僕は自分の言葉に置き換えてみた。

「つまり……パイプに繋ぐ部品の規格を決めている、みたいなことですか?」

ロックさんの手が止まった。初めて、こちらを真っ直ぐ見た。

「それだよ、ワトソン君。JIS規格で口径が揃っているから、メーカーが違っても繋がる。requires 'process' はパイプラインのJIS規格だ」

ステージの実装 — 五つの部品

「では、五百行の神メソッドの最初のステップ——バリデーションをステージにする」

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
package Stage::Validate;
use v5.36;
use Moo;
use Carp qw(croak);
with 'Role::Stage';

sub process ($self, $data) {
    croak 'order_id is required'
        unless defined $data->{order_id};
    croak 'items must be an array'
        unless ref $data->{items} eq 'ARRAY';
    croak 'items must not be empty'
        unless $data->{items}->@*;
    croak 'tax_rate is required'
        unless defined $data->{tax_rate};
    return $data;
}

「あ……これだけですか」

「これだけだ。バリデーションの仕事は、入力が正しいか確認して、正しければそのまま返すこと。正しくなければ死ぬこと。他のことは一切やらない

「……50行もないですね」

ロックさんが次のステージを壁に書き加えた。

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

sub process ($self, $data) {
    for my $item ($data->{items}->@*) {
        $item->{name}  =~ s/\A\s+|\s+\z//g;
        $item->{price} = int($item->{price});
    }
    return $data;
}
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
package Stage::Calculate;
use v5.36;
use Moo;
with 'Role::Stage';

sub process ($self, $data) {
    my $subtotal = 0;
    for my $item ($data->{items}->@*) {
        $subtotal += $item->{price} * $item->{quantity};
    }
    $data->{subtotal} = $subtotal;
    $data->{tax}      = int($subtotal * $data->{tax_rate});
    $data->{total}    = $subtotal + $data->{tax};
    return $data;
}
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
package Stage::Format;
use v5.36;
use Moo;
with 'Role::Stage';

sub process ($self, $data) {
    my @lines;
    for my $item ($data->{items}->@*) {
        push @lines, sprintf '%s x%d  ¥%d',
            $item->{name}, $item->{quantity},
            $item->{price} * $item->{quantity};
    }
    push @lines, sprintf '小計: ¥%d', $data->{subtotal};
    push @lines, sprintf '税額: ¥%d', $data->{tax};
    push @lines, sprintf '合計: ¥%d', $data->{total};
    $data->{formatted} = join "\n", @lines;
    return $data;
}
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
package Stage::Notify;
use v5.36;
use Moo;
with 'Role::Stage';

sub process ($self, $data) {
    $data->{notified} = 1;
    $data->{notification_body} = sprintf
        "ご注文ありがとうございます。\n\n%s",
        $data->{formatted};
    return $data;
}

五つのステージが並んだ。どれも短い。メモ帳に書き写しながら、不思議な感覚を覚えた。

どのステージも、自分の仕事しかしていない。バリデーションは検証だけ。正規化は整形だけ。計算は計算だけ。——あの500行のメソッドの中で絡み合っていた処理が、こうも綺麗に分かれるものなのか。

「五つのステージ……確かに五つですね。でも、これをどうやって繋ぐんですか?」

Pipeline クラス — 流れを作る

「ここが核心だ」

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

has stages => (
    is       => 'ro',
    isa      => ArrayRef[ConsumerOf['Role::Stage']],
    required => 1,
);

sub execute ($self, $data) {
    my $result = $data;
    for my $stage ($self->stages->@*) {
        $result = $stage->process($result);
    }
    return $result;
}

ConsumerOf['Role::Stage']?」

Role::Stagewith で消費しているクラスだけを配列に入れられる、という型制約だ。JIS規格を満たしていない部品は、パイプに接続させない」

僕は自分の言葉で咀嚼した。

「つまり……パイプラインは、ステージの配列を受け取って、最初から最後まで順番に process を呼ぶだけ……ということですか」

「それだけだ。パイプラインは何も知らない。バリデーションの中身も、計算ロジックも知らない。ただ順番に流すだけだ。知らないことが強さなのだよ、ワトソン君」

「でも……ステージの間でデータの形が変わったりしませんか? バリデーションの出力と計算の入力って、同じ形なんでしょうか」

ロックさんが机上のエルボ継手を手に取り、僕に手渡した。ずしりと重い。

「いい疑問だ。パイプラインの『パイプ』には口径がある。このエルボは方向を変えるが、口径は入口も出口も同じだ」

手の中で銅の部品を回した。確かに、両端の口径は同じだった。

「全ステージの入出力はハッシュリファレンス——同じ口径——で統一する。各ステージは受け取ったハッシュに自分の責任範囲のキーだけを加工し、そのまま次に渡す。全ステージが同じ口径のパイプで繋がっている

「同じ口径……。だから、どのステージをどの順番に繋いでも動くんですね」

ロックさんが壁のチョーク図に戻り、フィルターとフィルターの間に「+」を書き込んだ。

「割引計算のあとにログ出力を追加したくなったら?」

「ステージを一つ作って、配列に挿入すれば……」

「Pipeline の stages に1つ足すだけだ。process_order は存在しない。呼び出し元は何も変わらない

僕は前のめりになっていた。声に力が入っているのが自分でもわかった。

「あ——それだ。メソッドを分けても、呼び出す順番は process_order の中に固定されますよね。でもパイプラインなら、順番はステージの配列で決まるから……」

「正解だ、ワトソン君。分割と合成は違う。Extract Method は分割だ。Pipeline は合成可能な分割だ」

組み立て — 配管を繋ぐ

「実際に繋いでみよう」

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
use Pipeline;
use Stage::Validate;
use Stage::Normalize;
use Stage::Calculate;
use Stage::Format;
use Stage::Notify;

my $pipeline = Pipeline->new(
    stages => [
        Stage::Validate->new,
        Stage::Normalize->new,
        Stage::Calculate->new,
        Stage::Format->new,
        Stage::Notify->new,
    ],
);

my $result = $pipeline->execute($order_data);

「これが、五百行の迷路の出口だ」

壁のチョーク図と、画面のコードを見比べた。同じ形をしていた。入力から始まり、五つのステージを通り、出力に至る。

事件の終わり — テストの入口が開く

僕は、ずっと言えなかったことを口にした。

「テスト……書けるようになりますか?」

自分でも声が震えているのがわかった。上司から「テストを書いてから直せ」と言われたのが、ここに来た直接の理由だった。

ロックさんが壁の配管図を指しながら答えた。

「書ける。もう書ける」

 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
use Test::More;
use Test::Exception;

# Stage::Validate の単体テスト
my $validate = Stage::Validate->new;

throws_ok {
    $validate->process({});
} qr/order_id is required/, 'order_id がなければ死ぬ';

throws_ok {
    $validate->process({ order_id => 1 });
} qr/items must be an array/, 'items がなければ死ぬ';

# Stage::Calculate の単体テスト
my $calc = Stage::Calculate->new;
my $out  = $calc->process({
    items    => [{ price => 1000, quantity => 2 }],
    tax_rate => 0.1,
});
is $out->{subtotal}, 2000, '小計は正しい';
is $out->{tax},      200,  '税額は正しい';
is $out->{total},    2200, '合計は正しい';

done_testing;

「Stage::Validate はバリデーションだけを行う。不正な入力を渡して、正しくエラーになるか確認すればいい。Stage::Calculate は計算だけを行う。既知の入力を渡して、期待する出力と比較すればいい」

ロックさんがこちらを見た。

「五百行のメソッドはテストできなかった。だが五十行のステージは五分でテストが書ける。パイプラインの力はここだ。テストの入口が五つに増える

僕は画面を見つめた。テストコードは短かった。Stage::Validate のテストは、入力を渡して、期待するエラーメッセージが出るか確認するだけ。Stage::Calculate のテストは、入力を渡して、出力の数値を比較するだけ。

「……五分で書ける」

しばらく黙った。

テストが書けない恐怖。それは、五百行のメソッドのどこが何をしているかわからない恐怖だった。どこを触ったら何が壊れるかわからないから、何も触れなかった。でも、ステージに分ければ——何をテストすればいいかが、自明になる。

「……すみません。ちょっと、ほっとして」

ロックさんの声が、少しだけ柔らかくなった。

「ほっとするのは、パイプラインが通った証だよ、ワトソン君」

ロックさんがマグカップのコーヒーを飲み干し、立ち上がった。

「報酬の話だが」

「あ、はい。料金はどちらに——」

「料金はいらない。代わりに——配管工の名を冠したイタリア人の冒険譚だ。赤い帽子のほうを一冊」

「……マリオ、ですか?」

「水道管の中を走り回る男が、何十年も世界を救い続けている。パイプラインの力を侮るなよ、ワトソン君」

それはゲームでは……。 言いかけたけれど、ロックさんはもう壁の配管図に向き直っていた。僕は「は、はあ……」と困惑しつつ、荷物をまとめてドアに向かった。

薄暗い階段を降りながら、process_order の500行を頭の中で区切った。

バリデーション。正規化。計算。整形。通知。五つ。……たった五つだ。

五百行が怖かったのは、どこからどこまでが何の仕事か、わからなくなっていたからだ。壁がなかったから。でも、壁を作ればいい。パイプで繋げばいい。

テストが書けなかったのは、入口がなかったからだ。入口が五つになれば、一つずつ確かめていける。

……明日、まず Stage::Validate から書こう。五分で書けるなら、始めるのに五分もかからない。

1階の不動産屋の前を通りかかると、おじさんが顔を上げた。

「解決した?」

小さく頷いた。「はい。……配管の話でした」

おじさんが怪訝な顔をしている。その視線を背に受けながら外に出た。

曇り空の下、僕はスマホではなくメモ帳を開いた。「Stage::Validate → Stage::Normalize → Stage::Calculate → Stage::Format → Stage::Notify」と書いて、五つの箱を矢印で繋いだ。パイプラインの設計図。

ロックさんが壁に描いていた配管の図と、同じ形だ。


探偵の調査報告書

容疑(アンチパターン)真実(パターン)証拠(効果)
神メソッド(God Method)— 500行の密結合Pipeline Pattern — ステージ分割と順次実行各ステージが独立テスト可能に
処理ステップの暗黙的結合 — 共有ローカル変数Stage Role — requires 'process' でインターフェース保証ステージ追加・削除が呼び出し元に影響しない
テスト不能 — 入口が1つしかない各ステージの単体テストテストの入口が5つに増加

推理のステップ

  1. 神メソッドの処理ステップを境界コメントで可視化する
  2. 各ステップを独立した Stage クラスとして抽出する
  3. Moo::Role で Stage インターフェース(requires 'process')を定義する
  4. 各 Stage クラスが Role を消費し、process メソッドを実装する
  5. Pipeline クラスを作り、Stage 配列を execute で順次実行する
  6. 各 Stage の単体テストを書き、テストが通ることを確認する
  7. 統合テスト: Pipeline 全体を通した入出力を検証する

ロックより

パイプラインは「何も知らない」ことが強さだ。バリデーションの中身も、計算の仕組みも、通知の方法も知らない。ただ、ステージを順番に繋いで流すだけだ。

五百行の迷路が怖かったのは、壁がなかったからだ。壁を作り、パイプで繋げば、迷路は通路になる。一本ずつ確かめていける。

明日からは、ステージを一つ切り出すところから始めたまえ。五分でテストが書けるなら、始めるのに五分もかからない。——さあ、配管工の冒険譚を忘れないでくれたまえよ、ワトソン君。

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