0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

【どう実装する?シリーズ #1】「このパラメータ、あのパラメータと同時には使えないんだよね」という仕様

Posted at

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

振り返って、こうしておけばよかったと思う設計を示します。

基本方針

  1. Strategyパターンで各アクションタイプのロジックをカプセル化
  2. Enumでキーとアクションタイプを型安全に管理
  3. 各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を使うことで、これらの問題を未然に防ぎ、拡張に強い設計にできました(正確には「できたはず」ですが...)。

次回は、この設計にさらに「履歴データの保存」機能を追加したときの話を書く予定です。

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?