Laravel で Fat Controller を防ぐ 5 つの Tips

この記事について

Laravel は Policy や FormRequest など、Controller を補佐するモジュールを多く提供しているので Fat Controller になりにくいとは思いますが、それでもときどき Fat Controller に出くわすことがありますので、それらを防ぐために使えるテクニックというか、Tips を5つばかり紹介したいと思います。

はじめに

Fat Controller とは？

簡潔にいえば、行数が多く（個人的には Controller だとひとつのメソッドが NCLOC で 50 行を超えると Fat 感を感じます、みなさんはいかがでしょうか）、行数が多いがために処理の流れを追うことが難しく、しばしば不具合の原因になるクラスのことをいうのだと思います。本記事では、ただ行数が多いというだけでなく、複数のメソッドで処理が重複していたり、メソッドの中で条件分岐が発生したりして、ひとつの変更が他の部分に及ぼす影響を検知しづらい状態にあるコントローラーを想定しています。

巨大なクラスであっても、そこに含まれるデータや関数が高凝集・低結合であれば問題ないと考えます。大事なのは「不具合の原因になる」ということであって、すべての巨大なクラスが悪である、ということではないと思っています。

Fat Controller の問題点

【長すぎるメソッド】ひとつひとつのメソッドが長く、処理の流れを追うことが難しい。それによって、局所的な変更が処理全体に影響することがあり、予期せぬ不具合が発生することがある
【処理の重複】Controller 間で処理が重複することによって、仕様変更によるコードの変更が漏れ、予期せぬ不具合が発生することがある
【単一責任原則違反】複数のパターンを同一の Controller で処理することによって、ひとつのパターンに関する変更が他のパターンへ影響することがあり、それによって予期せぬ不具合が発生することがある

Fat Controller になる主な要因として、Model 層が貧弱すぎる、ということが考えられます。本来 Model に書くべき処理を Controller に書いてしまっており、結果的に可読性が悪くなったり、ひとつのユースケースに対する変更が他のユースケースに影響を及ぼしたり、Controller 間でのコピペコードを量産したり、ということが起きます。

Fat Controller を防ぐ 5 つの Tips

リクエストのデータを処理する関数は FormRequest に書く
レスポンスのデータを処理する関数は ViewModel あるいは Resource に書く
シングルアクションコントローラーにする
複数の Controller に分離する
UseCaseInteractor を使う

1. リクエストのデータを処理する関数は FormRequest に書く

public function search(Request $request) {
    if (isset($request->query('name'))) {
        $whereParams['name'] = $request->query('name');
    }
    return User::where($whereParams)->get();
}

みたいなリクエストパラメータの有無によって処理が分岐していくパターンです。上の例はひとつだけですが、これがずらっとあったり、if 文の中の条件が複雑になってくるとだいぶ読みにくくなってきます。

そこで、これらの処理は FormRequest 側へ移します。

// Controller
public function search(SearchUserRequest $request) {
    return User::where($request->filters())->get();
}

// FormRequest
public function filters(): array {
    $filters = [];
    if (isset($this->query('name'))) {
        $filters['name'] = $this->query('name');
    }
    return $filters;  
}

処理をただ他のクラスに移しただけじゃねーか、と思われるかもしれませんが、FormRequest へ移動することのメリットは、モデルに渡す入力パラメータの構築にのみ注力できる、ということで、たとえば、バリデーションルールは FormRequest に書いてあるでしょうから、入力パラメータが増えたりしたときに、FormRequest のみを変更すればよい状態にしておくと、変更漏れが起こりにくいんじゃないかと思います。また、複数のモデルに対して連続して処理するような変更が起こった場合にも、どれがどのモデルに関連するパラメータなのか、区別がつきやすくなると思います。

public function search(SearchUserRequest $request) {
    $users = User::where($request->userFilters())->get();
    $anotherModels = AnotherModels
        ::whereIn('user_id', $users->pluck('id'))
        ->where($request->anotherFilters())
        ->get();
    return ['users' => $users, 'anotherModels' => $anotherModels];
}

2. レスポンスのデータを処理する関数は ViewModel あるいは Resource に書く

public function index() {
    $users = User::where(...)->get();
    foreach ($users as $user) {
        $user->full_name = $user->family_name . ' ' . $user->given_name;
    }
    return view('user.index', compact('users'));
}

上の例はアクセサでやるほうがいいかなと思いますが、いい例を思いつかなかったのでご容赦を。

要は、Controller で受け取った Model からの戻り値を、View に渡す前に加工しなければならないケース、ということです。

// Controller
public function index() {
    $users = User::where(...)->get();
    $viewModel = new UsersViewModel($users);
    return view('user.index', ['users' => $viewModel->users()]);
}

// ViewModel
public function __construct(array $users) {
    $this->users = $users;
}
public function users(): array {
    return array_map([$this, 'transform'], $this->users);
}
private function transform(User $user): array {
    return [
        'id'   => $user->id,
        'name' => $user->family_name . ' ' . $user->given_name,
    ];
}

transform が返すのはオブジェクトにしてもいいでしょう。変換ロジックを書き換えたければ、 Controller 側で差し替えられるようにすると柔軟性が出ます。

// Controller
$viewModel = new UsersViewModel($users, new SummaryUserTransformer);

// ViewModel
public function __construct(array $users, UserTransformerInterface $transformer = null) {
    $this->users = $users;
    $this->transformer = $transformer ?? new DefaultUserTransformer;
}
public function users(): array {
    return array_map($this->transformer, $this->users);
}

// Transformer
public function __invoke(User $user) {
  return [
      // ...
  ];
}

3. シングルアクションコントローラーにする

シングルアクションコントローラーとは、 __invoke メソッドを持つ、特定のルーティングと対になる Controller クラスのことです。

ルーティングの書き方に特徴があります。

通常は Route::get('/', 'HomeController@index') のように @ の前にクラス名、後にメソッドを書きますが、シングルアクションコントローラーの場合は Route::get('/', 'HomeController') のようにクラス名のみ指定します（呼び出し時には自動的に __invoke が呼ばれます）。

いわゆる CRUD と呼ばれる処理は、 index/create/store/show/edit/update/delete の中から必要なメソッドを選択して使えばいいですが、それ以外のメソッドは、シングルアクションコントローラーにしておくと、クラスがスッキリすると思います。

4. 複数の Controller に分離する

複数の異なるユースケースを1つのアクションメソッドにまとめてしまうと、パターンによる条件分岐が必要になるため、Fat Controller になりがちです。

もし、Controller 内に、リクエストに応じて処理を分ける、みたいな条件分岐があるようであれば、複数の異なるユースケースをまとめてしまっている可能性があるので、分離できないか検討してみます。

public function __invoke(SearchUserRequest $request) {
    if ($request->is_special) {
        $users = User::searchSpecial($request->specialFilters())->get();
    } else {
        $users = User::search($request->filters())->get();
    }
    // ...
}

これを2つの Controller に分離してしまいます。

// SpecialContext\SearchUserController
public function __invoke(SearchUserRequest $request) {
    $users = User::searchSpecial($request->filters())->get();
    // ...
}
// GenericContext\SearchUserController
public function __invoke(SearchUserRequest $request) {
    $users = User::search($request->filters())->get();
    // ...
}

必要なら Model も分けてしまってもいいかもしれません。

// SpecialContext\SearchUserController
// use SpecialContext\User
public function __invoke(SearchUserRequest $request) {
    $users = User::search($request->filters())->get();
    // ...
}
// GenericContext\SearchUserController
// use GenericContext\User
public function __invoke(SearchUserRequest $request) {
    $users = User::search($request->filters())->get();
    // ...
}

クラスは同じで「別のアクションメソッドに分離する」というのもオッケーですが、CRUD 以外はシングルアクションコントローラーにする、というルールにするなら、Controller を分けることになるでしょう。

5. UseCaseInteractor を使う

UseCaseInteractor はクリーンアーキテクチャの一部で、アプリケーションへの入力（リクエスト）を受け取って中核となる処理を行い、ビュー（あるいは API レスポンス）へのアウトプットを生成するクラスです。

参考）Laravelでクリーンアーキテクチャ - Qiita #UseCaseInteractor

上の例では UseCaseInteractor は値を戻さず、Middleware のプロパティに直接渡す方式を取っていますが、本記事ではそのまま戻すようにしています（名前も簡略化して UseCase としています）。

// Controller
public function __invoke(SearchUserUseCase $useCase, SearchUserRequest $request) {
    $users = $useCase->invoke($request->filters());
    $responder = new UsersResponder($users);
    return $responder->createResponse();
}
// UseCase
public function invoke(array $filters) {
    return User::where($filters)->get();
}
// Responder with Blade view
public function createResponse() {
    $viewModel = new UsersViewModel($this->users, new SummaryUserTransformer);
    return view('user.index', ['users' => $viewModel->users()]);
}
// Responder with API resource
public function createResponse() {
    return UserResource::collection($this->users);
}

UseCase 内に処理を閉じ込めることで、Controller をスリムに保つことができます。このクラスに渡すデータは、HTTP の世界の情報はプリミティブなデータ、あるいはドメインオブジェクトに変換しておくのがいいと思います。

おわりに

いかがでしたでしょうか。例がシンプルすぎていまいちピンとこないかもしれませんが、Fat Controller を解消したいとお悩みの方の一助になれば幸いです。

他にも Fat Controller 解消法をお持ちの方、コメント欄にて教えていただけると大変助かります

次回は「Laravel で Fat Model を防ぐ 5 つの Tips」をお送りしたいと思います。

2020-02-17 10:38 追記
大事なことが抜けていたので補遺を書きました。
Laravel で Fat Controller を防ぐもうひとつの Tip - Qiita
追記ここまで