Featured image of post コード探偵ロックの事件簿【Parameter Object】終わらないバケツリレーの悲劇 〜手荷物検査のない引数の海〜

コード探偵ロックの事件簿【Parameter Object】終わらないバケツリレーの悲劇 〜手荷物検査のない引数の海〜

「引数が多すぎて順番を間違えた!」。いくつもの関数を跨いでただ変数を引き回すだけの「終わらないバケツリレー」。それは Data Clumps(データの群れ)と Long Parameter List(長い引数リスト)が引き起こす悲劇。Parameter Object で荷物をカバンに詰め込み、引数の迷宮から脱出しましょう。

I. 依頼(終わらないバケツリレー)

「バグの原因が全く分かりません! 注文処理の奥深くで、絶対存在しているはずのユーザーIDのエラーが出るんです。呼び出し元の引数は合っているはずなんですが……!」

深夜の「レガシー・コード・インベスティゲーション(LCI)」。 ECサイトの注文処理システムを担当する若手エンジニアの私(ワトソン君)は、目の下に濃い隈を作りながら、探偵事務所のドアを叩いた。

「ふむ。君は真面目すぎるがゆえに、荷物をただ隣の人に渡し続けるだけの『終わらないバケツリレー』を構築してしまったようだな」

コード探偵ロックは、冷えきったピザの欠片を齧りながら、私の持ち込んだノートPCの画面に目を落とした。

「バケツリレー?」

「『権限フラグも欲しい』『キャンペーンコードも追加で』という仕様追加の嵐に愚直に応え続けた結果だろう。君のコードは、引数が10個もある関数群が5階層も続くという、壮大な迷宮を完成させてしまっているじゃないか」

II. 現場検証(無実の罪と手荷物検査)

延々と続く引数のバケツリレー。関数から関数へ、ただ運ばれるだけの変数たち

モニターには、私が連日追記を重ねて育て上げた注文処理ロジックが映し出されていた。

 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
# Before: 終わらないバケツリレー(Long Parameter List / Data Clumps)

# 呼び出し元
process_order($user_id, $shop_id, $item_id, $amount, $campaign_code, $auth_token);

# 第1階層
sub process_order {
    my ($self, $user_id, $shop_id, $item_id, $amount, $campaign_code, $auth_token) = @_;
    # 自分は使わないが、下層のためにただ引き回す
    return $self->validate_order($user_id, $shop_id, $item_id, $amount, $campaign_code, $auth_token);
}

# 第2階層
sub validate_order {
    my ($self, $user_id, $shop_id, $item_id, $amount, $campaign_code, $auth_token) = @_;
    
    die "Invalid auth token" unless $auth_token eq 'secret_token';
    # ... 中略 ...

    # 👇 ここで次の階層へ渡す
    return $self->calculate_discount($shop_id, $user_id, $item_id, $amount, $campaign_code);
}

# 最終階層(実際に値を使う場所)
sub calculate_discount {
    my ($self, $user_id, $shop_id, $item_id, $amount, $campaign_code) = @_;
    
    # 💥ここで悲劇が!
    # 呼び出し元で $user_id と $shop_id の指定順が逆になっていたため
    # DB検索でエラーになるか、意図しないユーザとして処理されてしまう!
    
    my $user_is_premium = ($user_id == 1) ? 1 : 0;
    # ...
}

「関数の引数が多いのは認めます。でも、下層の calculate_discount でどうしても $user_id$shop_id が両方必要だったんです。だから、その上の階層からはただ渡しているだけで……」

言い訳をする私の言葉を遮り、ロックがコードの第2階層—— validate_order メソッドの末尾を指差した。

「ほら、ここを見たまえ。第2引数の $user_id と第3引数の $shop_id、どちらも数値型だが……渡す順番が逆になっているぞ

「えっ!?」

私は画面に釘付けになった。 第2階層で $shop_id$user_id を渡す順番を間違えていたせいで、第3階層の $user_id には $shop_id の数値が入ってしまっていたのだ。両方ともただの数値(Int型)だから、文法エラーにはならずに実行できてしまっていた。

「そんな……。引数が多すぎて、しかもただ次の関数にパスするだけだったから、コピペした時にズレたんだ……!」 私が頭を抱えると、ロックは呆れたように肩をすくめた。

「関数の引数は『その関数が直接使うもの』だけを受け取るべきだ。使わないものをただ運ぶだけのバケツリレーは、途中に関心がなく監視の目(手荷物検査)がない分、今回のように簡単に中身がすり替わるのさ」

III. 推理披露(カバンに詰めてラベルを貼れ)

ロックはピザの油をティッシュで拭き取ると、キーボードに手を乗せた。

「こんなバラバラの荷物(引数)を両手に10個も抱えて歩くから落とすんだ。常に一緒に連れ回されている変数群(Data Clumps / データの群れ)を見つけ出し、一つの「カバン」に詰めなさい。それが今回の解決策、Parameter Object(引数オブジェクト) だ」

「カバン……引数をオブジェクトにまとめるってことですか?」

「その通り。さっそく専用のクラス(Mooオブジェクト)を作成しよう」

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
# After: カバン(Parameter Object)の導入

package OrderContext;
use Moo;
use Types::Standard qw(Int Str Optional);

# 常に一緒にいるデータ群(Data Clumps)をクラスにまとめる
has user_id       => (is => 'ro', isa => Int, required => 1);
has shop_id       => (is => 'ro', isa => Int, required => 1);
has item_id       => (is => 'ro', isa => Int, required => 1);
has amount        => (is => 'ro', isa => Int, required => 1);
has campaign_code => (is => 'ro', isa => Optional[Str]);
has auth_token    => (is => 'ro', isa => Str, required => 1);

1;

ロックは素早く OrderContext という「カバン」を作成した。 そして、5階層に及ぶ関数群の引数を、すべてこの $context オブジェクト1つに変更し始めたのだ。

 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
# 呼び出し元:バラバラの荷物をカバン(Context)に詰めて渡す
# 名前付き引数として渡すため、順番間違いが言語仕様レベルで起きない
my $context = OrderContext->new(
    user_id       => $user_id,     # ラベルが付いているので安全
    shop_id       => $shop_id,
    item_id       => $item_id,
    amount        => $amount,
    auth_token    => $auth_token,
    campaign_code => $campaign_code,
);

$service->process_order($context);

# 第1階層、第2階層
# 引数が1つになり、中継地点のシグネチャがすっきりする!
sub process_order {
    my ($self, $context) = @_;
    return $self->validate_order($context);
}

sub validate_order {
    my ($self, $context) = @_;
    die "Invalid auth token" unless $context->auth_token eq 'secret_token';
    return $self->calculate_discount($context);
}

# 最終階層
sub calculate_discount {
    my ($self, $context) = @_;
    
    # 本当に必要な時に、カバンから必要なものだけを取り出す
    my $user_is_premium = ($context->user_id == 1) ? 1 : 0;
    
    my $discount = 0;
    if (my $campaign_code = $context->campaign_code) {
        $discount = 500 if $campaign_code eq 'SUMMER';
    }
    # ...
}

「引数が……1つになった!」

私は歓声を上げた。 「名前付きでカバンに詰めるから順番を間違えようがないし、途中の関数も単に $context を渡すだけ。これなら、後から『クーポンIDも追加で』って言われても、カバン(Context)の中身を増やすだけで、途中の関数たちの定義(シグネチャ)をいちいち書き換える必要がないんですね……!」

IV. 解決(静かなる注文処理)

バグは完全に消え去り、私のコードは将来の仕様追加(引数追加)にも強い、堅牢でスッキリとした構造に生まれ変わった。

「データは群れをなす性質がある。常に幾つかの変数が一緒にいるのを見つけたら、彼らに『名前(クラス)』を与え、一つのチームとして扱うのだよ」 ロックは満足げに、そしてドヤ顔で言い放った。

「なるほど! 理解しました。じゃあこれからは、どんな関数でも引数は全部1つの巨大な GlobalContext にまとめちゃえば最強ですね! 引数は常に1つ! これで解決です!」

私が自信満々に極論を導き出すと、ロックは眉間を揉み解しながら呻いた。

「……それはただのグローバル変数の再発明だ、ワトソン君。何事も『適切な粒度』というものがある。カバンが大きすぎれば、今度は中身を探すのに苦労することになるんだからね」


探偵の調査報告書

容疑(アンチパターン)真実(パターン)証拠(効果)
Long Parameter List (長い引数リスト) / Data Clumps (データの群れ)。常に同じ変数の組み合わせ(IDやトークンなど)が、関数から関数へ長々と引き回されている状態。Introduce Parameter Object (引数オブジェクトの導入)。群れをなしている変数を、それを表現する1つのクラス(オブジェクト)にまとめる。データの凝集性が向上する。関数の引数が減ることで順番指定ミスが起きなくなり、パラメータ追加時の変更波及範囲(シグネチャの修正)を最小限に抑えられる。

推理のステップ

  1. データの群れ(Data Clumps)を特定する 複数の関数の引数に、常に同じパラメータの組み合わせが存在していないか確認する。(例: $user_id, $shop_id, $amount など)
  2. 専用のクラス(Parameter Object)を作成する それらのパラメータをプロパティとして持つ新しいクラスを作成する。
  3. 引数をオブジェクトに置き換える 元の関数の長い引数リストを、作成したインスタンス1つに置き換える。途中の関数の中身は触らず、本当に値が必要な末端の関数でのみオブジェクトから値(プロパティ)を取り出す。

ロックより

使わないものを無自覚に運ばせるな。運ぶなら、名前のついたカバンに入れろ。 システムはバケツリレーの現場ではない。君が渡しているのは水ではない、意味のあるデータなのだから。

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