今回は、あちこちに処理が散らばっていたレガシーなコードを、3つの関数(form、confirm、submit)に分割してスッキリ整理したリファクタリングの実践例をご紹介します。
1. 修正前のコードが持つ「2大アンチパターン」
まずは、リファクタリング前のコードを見てみましょう。一見すると動いていますが、処理を追うのが非常に困難な構造をしています。
switch ("$requestMethod.$action") {
case 'GET.':
$form = [];
$_SESSION[APPID]['form'] = [];
$tpl = 'form.tpl';
break;
case 'GET.form':
$form = $_SESSION[APPID]['form'] ?? [];
$tpl = 'form.tpl';
break;
case 'POST.confirm':
$form = $_POST;
$tpl = 'confirm.tpl';
break;
case 'POST.submit':
$form = $_SESSION[APPID]['form'] ?? [];
$tpl = 'thanks.tpl';
break;
default:
die();
}
if (!isset($form['prefecture'])) {
$form['prefecture'] = '';
}
if (!isset($form['rating'])) {
$form['rating'] = '';
}
if (!isset($form['where_arr'])) {
$form['where_arr'] = [];
}
// {html_select_date/time}用タイムスタンプを計算する
$now = time();
if (isset($form['startDate'])) {
makeTimeStamp($form, ['field_array' => 'startDate']);
} else {
$form['startDate']['TimeStamp'] = $now;
}
if (isset($form['endDate_Year'])) {
makeTimeStamp($form, ['prefix' => 'endDate_']);
} else {
$form['endDate_TimeStamp'] = $now + 7 * 24 * 3600;
}
$smarty = new Smarty();
$smarty->assign('META', $META);
$smarty->assign('form', $form);
$smarty->display('pages/chapter4_7/' . $tpl);
switch ("$requestMethod.$action") {
case 'POST.confirm':
$_SESSION[APPID]['form'] = $form;
break;
case 'POST.submit':
$_SESSION[APPID]['form'] = [];
break;
}
このコードの何が辛いのか、アンチパターンは大きく以下の2点に集約されます。
(1) switch ➔ if ➔ if ➔ switch の構造が追跡しにくい
最初のエントリポイント(前半の switch)でルーティングやテンプレートを決めたはずなのに、ファイルの一番下でもう一度 switch が登場します。 1つのアクション(例:POST.confirm)のライフサイクルを追うために、ファイルの上から下へ視線を何度も往復させられる構造になっていました。
また途中の if は、新規の入力フォームでここが成立するのか?action=confirmでここが成立するのか?コードを読んでも理解が難しい箇所でした。
(2) $smarty->display の後で、セッション保存している
これが最も直感に反するポイントです。画面を描画する $smarty->display(...) が実行された「後」の後半 switch で、セッションへの書き込み($_SESSION[APPID]['form'] = $form;)を行っています。 「描画が終わった後に裏でデータが書き換わる」という隠れた副作用があるため、処理の順序ミスやバグの温床になりやすい危険な書き方です。
2. リファクタリング:「前処理 ➔ 処理 ➔ 表示」を関数に閉じ込める
このカオスな構造を解消するため、アクションごと(form、confirm、submit)に処理を3つの関数へ分割しました。共通化にはこだわらず、それぞれの関数内で完結させる方針にしています。
// ----- インライン関数定義 -----
/**
* @param array<string, mixed> $meta メタデータ
* @param array<string, mixed> $formFromSession セッションから復元したフォームデータ
*/
function form(array $meta, array $formFromSession = []): void
{
$now = time();
$defaultForm = [
'prefecture' => '',
'rating' => '',
'where_arr' => [],
'startDate' => ['TimeStamp' => $now],
'endDate_TimeStamp' => $now + 7 * 24 * 3600,
];
if (empty($formFromSession)) {
$form = $defaultForm;
$_SESSION[APPID]['form'] = [];
} else {
$form = array_merge($defaultForm, $formFromSession);
}
$smarty = new Smarty();
$smarty->assign('META', $meta);
$smarty->assign('form', $form);
$smarty->display('pages/chapter4_7/form.tpl');
}
/**
* @param array<string, mixed> $meta メタデータ
* @param array<string, mixed> $postVars POSTされたフォームデータ
*/
function confirm(array $meta, array $postVars): void
{
$form = $postVars;
makeTimeStamp($form, ['field_array' => 'startDate']);
makeTimeStamp($form, ['prefix' => 'endDate_']);
// 描画の「前」にセッション保存を完結させる
$_SESSION[APPID]['form'] = $form;
$smarty = new Smarty();
$smarty->assign('META', $meta);
$smarty->assign('form', $form);
$smarty->display('pages/chapter4_7/confirm.tpl');
}
/**
* @param array<string, mixed> $meta メタデータ
* @param array<string, mixed> $formFromSession セッションから復元したフォームデータ
*/
function submit(array $meta, array $formFromSession): void
{
// メイン処理・データ操作(未実装部分)
$_SESSION[APPID]['form'] = [];
$smarty = new Smarty();
$smarty->assign('META', $meta);
$smarty->assign('form', $formFromSession);
$smarty->display('pages/chapter4_7/thanks.tpl');
}
// ----- ルーティング(エントリポイント) -----
switch ("$requestMethod.$action") {
case 'GET.':
form($META);
break;
case 'GET.form':
form($META, $_SESSION[APPID]['form'] ?? []);
break;
case 'POST.confirm':
confirm($META, $_POST);
break;
case 'POST.submit':
submit($META, $_SESSION[APPID]['form'] ?? []);
break;
default:
die();
}
3. リファクタの効果:コードが語るようになってきた
実際に修正後の3つの関数を見比べてみると、「それほど複雑な処理ではない」ということが一目で分かります。今回のリファクタリングで、以下の効果が得られました。
- 処理の完結性: 各関数(
form,confirm,submit)の中に「前処理・データ準備 ➔ セッション保存等の副作用 ➔ テンプレート描画」の流れがすべて閉じ込められ、ファイル内を行ったり来たりする必要がなくなりました。 - 副作用の適正化: 描画の前にセッション保存を終える形になったため、コードの実行順序が直感的になり、バグの入り込む隙間が減りました。
- 見通しの良さ: エントリポイントの
switchは純粋な「ルーティング(どの関数を呼ぶか)」に特化し、責務が綺麗に分離されました。
まとめ:レガシーコードには「小さな関数分割」から
レガシーSmartyのような古いコードベースを前にすると、すべてをモダンなフレームワークに置き換えたくなりますが、コストやリスクの面で現実的ではない場合がほとんどです。
まずは「ファイルを上から下に追いかけなければならない構造」を断ち切り、アクションごとの関数に処理を閉じ込める。それだけでも、コードの見通しは劇的に改善します。
1リファクタ、1スコープ。 コードが読みやすくなると、次のアラが見えてきます。 次のアラを修正したくなっても、ぐっと我慢して、PRを作ってマージしましょう。
レガシーコードの担当者になってしまったエンジニアの皆さんの、小さなリファクタリングのヒントになれば幸いです。
関連PR


