前回はシンプルなif文でコマンド実行を実装しました。
「シンプル・イズ・ベスト」という言葉はありますが、拡張性を考慮しないシンプルさは、時として将来の負債となります。
今回は、あえて開発現場でよくある「機能追加の要求」を受け入れ続け、コードがどのように破綻していくかを見てみましょう。いわゆる「アンチパターン」の鑑賞回です。
追加要件のラッシュ
上司や同僚から、次々と新しい機能のリクエストが来たとします。
- ログ取得:
/log error --lines 50 でエラーログを直近50行見たい - DBクエリ:
/sql "SELECT * FROM users LIMIT 1" でReadOnlyユーザーでクエリしたい - ユーザー管理:
/user add nobu でSSHユーザーを追加したい - エイリアス:
/d でも /deploy と同じ動きにしてほしい
これらを前回の if-elsif 構造にそのまま追記していくとどうなるでしょうか？
崩壊するコード
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
| sub handle_message {
my ($text, $user_role) = @_; # 権限制御のためにroleも引数に追加...
$text =~ s/^\s+|\s+$//g;
# デプロイ処理（エイリアス追加）
if ($text =~ m{^/(?:deploy|d)\s+(\w+)(?:\s+(--force))?}) {
my ($target, $force) = ($1, $2);
# 権限チェックロジックが混入
if ($target eq 'production' && $user_role ne 'admin') {
return "⛔ 管理者権限が必要です";
}
# ロジック本番...
return cmd_deploy($target, $force);
}
# ログ取得処理（オプション解析が混入）
elsif ($text =~ m{^/log\s+(\w+)(?:\s+--lines\s+(\d+))?}) {
my ($level, $lines) = ($1, $2 // 10);
return cmd_log($level, $lines);
}
# SQL実行（危険な香りがする正規表現）
elsif ($text =~ m{^/sql\s+"(.+)"}) {
my $query = $1;
# SQLインジェクション対策簡易チェック...
if ($query =~ /DROP|DELETE/i) {
return "💥 破壊的なクエリは禁止です";
}
return cmd_sql($query);
}
# ユーザー追加
elsif ($text =~ m{^/user\s+add\s+(\w+)}) {
# ... もう読みたくないですよね？
}
# ... 以下、延々と続く
}
|
何が問題なのか？
このコードには、保守性を下げるいくつもの「不吉な臭い（Code Smell）」が漂っています。
1. 単一責任の原則 (SRP) 違反
handle_message 関数が多すぎることに関心を持ちすぎています。
- コマンドのルーティング（どれを実行するか）
- パラメータの解析（正規表現）
- 権限のチェック
- 引数のバリデーション
これらが一つの関数に詰め込まれているため、どれか一つを変更しようとすると、他の部分に影響が出るリスクがあります。
2. 開放閉鎖の原則 (OCP) 違反
新しいコマンドを追加するたびに、この handle_message 関数自体を修正しなければなりません。テスト済みの既存コードを触るということは、バグを混入させるチャンスを作ってしまうということです。
3. テスト困難
この関数をテストするには、あらゆるパターンのメッセージ文字列を用意しなければなりません。「SQLコマンドのロジックだけテストしたい」と思っても、ルーティング部分と密結合しているため単体テストが書きにくくなっています。
解決の糸口：Commandパターン
この状況を打破するための武器、それが 「Commandパターン」 です。
次回からは、「命令（コマンド）」を一つのクラス（オブジェクト）としてカプセル化し、このスパゲッティコードを美しく整理整頓していきます。
混沌とした状況こそ、リファクタリングの最大のチャンスです。次回、大掃除を始めましょう！
今回の破綻コード（アンチパターン）
今回は「完成コード」ではなく、要件追加によって複雑化し、リファクタリングが必要になった「破綻コード」を掲載します。
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
| # spaghetti_bot.pl
sub handle_message {
my ($text, $user_role) = @_;
$text =~ s/^\s+|\s+$//g;
# 1. デプロイ（エイリアス対応、強制オプション、権限チェック）
if ($text =~ m{^/(?:deploy|d)\s+(\w+)(?:\s+(--force))?}) {
my ($target, $force) = ($1, $2);
# 権限ロジックの混入
if ($target eq 'production' && $user_role ne 'admin') {
return "⛔ 管理者権限が必要です";
}
# バリデーションロジックの混入
my @allowed = qw(production staging);
unless (grep { $_ eq $target } @allowed) {
return "エラー: 無効な環境です";
}
return "🚀 $target 環境へのデプロイを開始しました" . ($force ? " (強制)" : "");
}
# 2. ログ取得（オプション解析）
elsif ($text =~ m{^/log\s+(\w+)(?:\s+--lines\s+(\d+))?}) {
my ($level, $lines) = ($1, $2 // 10);
return "📋 $level ログを直近 $lines 行取得しました...";
}
# 3. SQL実行（セキュリティチェック）
elsif ($text =~ m{^/sql\s+"(.+)"}) {
my $query = $1;
if ($query =~ /DROP|DELETE/i) {
return "💥 破壊的なクエリは禁止です";
}
return "📊 クエリ実行: $query";
}
# 4. ユーザー追加
elsif ($text =~ m{^/user\s+add\s+(\w+)}) {
# ...実装省略...
}
return "不明なコマンドです";
}
|
