CTA遷移先は「URL」「ステップ」「JS実行」のどれか1つだけ。こういう排他的パラメータの制御、Controller内でif文書いてませんか?実サービスで実際に苦しんだ設計を振り返り、「こうしておけばよかった」を考えます。
はじめに
私たちが開発しているLPcatsは、スワイプ型LPを作成・管理・改善するためのサービスです。今回はその中の「LP作成API」で実際に起きた機能追加を題材に、設計の失敗を振り返ります。
「あの時こうしておけば...」という反省を、同じような問題に直面している方と共有できればと思います。
LPcatsのデータ構造
まず前提となるデータ構造を説明します。
スワイプ型LPは以下の要素で構成されています:
- LP本体: LP全体の情報
- Step: LP内でスワイプされる各ページ
- Media: 各Stepに表示される画像や動画
- Parameter: LPやStepの挙動を決める設定値
Parameterは以下のようなテーブル構造です(EAVパターン):
parameters テーブル
- lp_id (必須)
- step_id (Stepに紐づく場合のみ)
- key (string)
- value (string)
この設計により、カラム追加なしで柔軟にパラメータを追加できます。
最初の実装
LP作成APIは、最初こんな感じでした(Laravel):
class LpController extends Controller
{
public function store(Request $request)
{
// LPを作成
$lp = new Lp();
$lp->title = $request->title;
$lp->save();
// Stepを作成
foreach ($request->steps as $stepData) {
$step = new Step();
$step->lp_id = $lp->id;
$step->content = $stepData['content'];
$step->save();
// Mediaを作成
if (isset($stepData['media'])) {
$media = new Media();
$media->step_id = $step->id;
$media->url = $stepData['media']['url'];
$media->save();
}
}
// Parameterを作成
$parameter = new Parameter();
$parameter->lp_id = $lp->id;
$parameter->key = 'swipe_speed';
$parameter->value = $request->swipe_speed;
$parameter->save();
return response()->json($lp);
}
}
シンプルで、動いていました。問題が起きるまでは。
問題が発覚:排他的パラメータの追加
機能追加として「CTAボタンをクリックしたときの挙動」を設定できるようにしました。
具体的には:
-
URLに遷移 →
cta_urlパラメータが必要 -
特定のStepに遷移 →
cta_step_idパラメータが必要
そして重要なのが、これらは同時に設定できない(排他的) という仕様です。
最初の実装では、こんな感じでController内に書きました:
public function store(Request $request)
{
// ... LP、Step、Media作成
// CTA系パラメータの保存
if ($request->cta_action_type === 'url') {
// URLパラメータを保存
Parameter::create([
'lp_id' => $lp->id,
'key' => 'cta_action_type',
'value' => 'url'
]);
Parameter::create([
'lp_id' => $lp->id,
'key' => 'cta_url',
'value' => $request->cta_url
]);
// 排他的なパラメータ(cta_step_id)を削除
Parameter::where('lp_id', $lp->id)
->where('key', 'cta_step_id')
->delete();
} elseif ($request->cta_action_type === 'step') {
// Stepパラメータを保存
Parameter::create([
'lp_id' => $lp->id,
'key' => 'cta_action_type',
'value' => 'step'
]);
Parameter::create([
'lp_id' => $lp->id,
'key' => 'cta_step_id',
'value' => $request->cta_step_id
]);
// 排他的なパラメータ(cta_url)を削除
Parameter::where('lp_id', $lp->id)
->where('key', 'cta_url')
->delete();
}
// ... 他のパラメータ保存
}
動きはしましたが、明らかに嫌な予感がしました。
さらに追加:JavaScript実行
その後、「JavaScript実行」という3つ目の選択肢が追加されました。
public function store(Request $request)
{
// ...
if ($request->cta_action_type === 'url') {
// ...
// cta_step_idとcta_jsを削除
Parameter::where('lp_id', $lp->id)
->whereIn('key', ['cta_step_id', 'cta_js'])
->delete();
} elseif ($request->cta_action_type === 'step') {
// ...
// cta_urlとcta_jsを削除
Parameter::where('lp_id', $lp->id)
->whereIn('key', ['cta_url', 'cta_js'])
->delete();
} elseif ($request->cta_action_type === 'javascript') {
// JavaScript保存処理...
// cta_urlとcta_step_idを削除
Parameter::where('lp_id', $lp->id)
->whereIn('key', ['cta_url', 'cta_step_id'])
->delete();
}
// ...
}
もうお察しの通り、if/elseif地獄の完成です。
実際に困ったこと
この設計で実際に困ったことを整理します。
1. バリデーションの散らばり
FormRequestでは基本的なバリデーションしかできません:
class LpStoreRequest extends FormRequest
{
public function rules()
{
return [
'cta_action_type' => 'required|string',
'cta_url' => 'nullable|url',
'cta_step_id' => 'nullable|integer',
// ↑ 文脈依存のルール(「URLの時は必須」など)は書けない
];
}
}
結果、「cta_action_type=url なのに cta_url が空」みたいなチェックをController内でやる羽目になりました。
if ($request->cta_action_type === 'url' && empty($request->cta_url)) {
throw new ValidationException('cta_url is required');
}
2. クリーンアップ漏れ・過剰削除
排他制御のロジックが散らばっているため、以下のようなバグが実際に起きました:
- クリーンアップ漏れ: Step削除時に、そのStepに紐づくParameterが残り続ける
- 過剰削除: CTA変更時に、関係ないパラメータまで消してしまう
削除すべきキーのリストが人間の頭の中にしかなく、コードから読み取れない状態でした。
3. テストの肥大化
Controllerが全ての責務を持っているため、テストケースが爆発的に増えました:
public function test_url遷移のパラメータが正しく保存される() { ... }
public function test_step遷移のパラメータが正しく保存される() { ... }
public function test_js実行のパラメータが正しく保存される() { ... }
public function test_url遷移に変更したときstep_idが削除される() { ... }
public function test_step遷移に変更したときurlが削除される() { ... }
// ...エンドレス
しかもこれ全部、Controllerレベルのテストです。重いし遅い。
4. keyがただのstringで管理されている
->where('key', 'cta_step_id') // タイポの危険
->where('key', 'cta_url') // IDE補完効かない
リファクタリング時に「このキーどこで使われてるの?」と全文検索する羽目に。
5. 変更のたびにController全体が影響を受ける
新しいパラメータを追加するたびに、このメソッド全体を理解し直す必要がありました。Open/Closed原則の真逆です。
こうしておけばよかった:Strategyパターン + Enum
振り返って、こうしておけばよかったと思う設計を示します。
基本方針
- Strategyパターンで各アクションタイプのロジックをカプセル化
- Enumでキーとアクションタイプを型安全に管理
- 各Strategyが自分の責任範囲(必要キー、排他キー、バリデーション)を明示
Enumでキーを型安全に
enum ParameterKey: string {
case CTA_ACTION_TYPE = 'cta_action_type';
case CTA_URL = 'cta_url';
case CTA_STEP_ID = 'cta_step_id';
case CTA_JS = 'cta_js';
case SWIPE_SPEED = 'swipe_speed';
}
enum CtaActionType: string {
case URL = 'url';
case STEP = 'step';
case JAVASCRIPT = 'javascript';
public function getStrategy(): ParameterStrategy {
return match($this) {
self::URL => new CtaUrlStrategy(),
self::STEP => new CtaStepStrategy(),
self::JAVASCRIPT => new CtaJavaScriptStrategy(),
};
}
}
これで:
- タイポ防止
- IDE補完が効く
- リファクタリングが安全
Strategyインターフェース
interface ParameterStrategy {
public function validate(array $data): void;
public function save(Lp $lp, ?Step $step, array $data): void;
public function getRequiredKeys(): array;
public function getExclusiveKeys(): array;
}
各Strategyは:
- 自分が必要とするキー
- 自分が削除すべきキー(排他的なもの)
- バリデーションルール
を明示的に持ちます。
具体的なStrategy実装
class CtaUrlStrategy implements ParameterStrategy {
public function validate(array $data): void {
if (empty($data[ParameterKey::CTA_URL->value])) {
throw new ValidationException('cta_url is required');
}
}
public function save(Lp $lp, ?Step $step, array $data): void {
// 排他的なパラメータを削除
$this->cleanupExclusiveParameters($lp, $step);
// 必要なパラメータを保存
$this->saveParameter($lp, $step, ParameterKey::CTA_ACTION_TYPE, CtaActionType::URL->value);
$this->saveParameter($lp, $step, ParameterKey::CTA_URL, $data[ParameterKey::CTA_URL->value]);
}
public function getRequiredKeys(): array {
return [ParameterKey::CTA_URL];
}
public function getExclusiveKeys(): array {
return [ParameterKey::CTA_STEP_ID, ParameterKey::CTA_JS];
}
private function cleanupExclusiveParameters(Lp $lp, ?Step $step): void {
// getExclusiveKeys()で明示されたキーを削除
// ...実装省略
}
}
Controllerはシンプルに
class LpController extends Controller
{
public function store(Request $request)
{
// LP、Step、Media作成...
// パラメータ保存
if (isset($request->cta_action_type)) {
$actionType = CtaActionType::from($request->cta_action_type);
$strategy = $actionType->getStrategy();
$strategy->validate($request->all());
$strategy->save($lp, null, $request->all());
}
return response()->json($lp);
}
}
if/elseif地獄が消えました。
新しいパラメータ追加時の変更
JavaScript実行機能を追加する場合:
1. Enumに追加
enum ParameterKey: string {
// ...
case CTA_JS = 'cta_js'; // ← 追加
}
enum CtaActionType: string {
// ...
case JAVASCRIPT = 'javascript'; // ← 追加
public function getStrategy(): ParameterStrategy {
return match($this) {
// ...
self::JAVASCRIPT => new CtaJavaScriptStrategy(), // ← 追加
};
}
}
2. 新しいStrategyクラスを追加(既存コードは変更なし)
class CtaJavaScriptStrategy implements ParameterStrategy {
// 必要なメソッドを実装...
}
3. 既存Strategyの排他キー配列に追加
class CtaUrlStrategy implements ParameterStrategy {
public function getExclusiveKeys(): array {
return [
ParameterKey::CTA_STEP_ID,
ParameterKey::CTA_JS, // ← 追加
];
}
}
変更箇所はこれだけ。Controllerは一切触りません。
この設計の利点
1. Open/Closed原則に準拠
新しいパラメータ追加は「拡張」であり、既存コードの「変更」ではありません。
2. 責務が明確
- バリデーション → 各Strategy
- 排他制御 → 各Strategyの
getExclusiveKeys() - 保存処理 → 各Strategy
どこに何が書いてあるか一目瞭然です。
3. テストが書きやすい
public function test_url_strategyは正しくバリデーションする() {
$strategy = new CtaUrlStrategy();
// Strategyクラス単体でテスト可能
}
Controllerの重いテストから解放されます。
4. 型安全
EnumのおかげでIDEの補完が効き、タイポも防げます。
5. 排他関係が明示的
getExclusiveKeys() を見れば、どのパラメータと排他かすぐ分かります。
まとめ
「排他的パラメータ」という一見シンプルな要件でも、素朴に実装すると:
- if/elseif地獄
- バリデーションの散らばり
- クリーンアップ漏れ
- テストの肥大化
といった問題が次々と発生します。
Strategyパターン + Enumを使うことで、これらの問題を未然に防ぎ、拡張に強い設計にできました(正確には「できたはず」ですが...)。
次回は、この設計にさらに「履歴データの保存」機能を追加したときの話を書く予定です。