【Laravel】組み合わせ検索の効率的な書き方について

Question

いつもお世話になっています。

表題について、みなさんだったらどのようにソースコードを書くかというのを教えていただけると幸いです。
現在Laravelにて組み合わせ【プール】×【勤務地】検索フォームの実装をしています。

自分は以下のソースコードで、ひとまず動くようにしたのですが、
コントローラの書き方が異常にブサイクで、もっとイケメンになりたいです。

特に困っている部分はSearchController.phpにおける、
**【 # ここからもっといい感じに書きたい】**とコメントが書いてある行以下のif文になります。

ここまでは何とかして、無事検索も動くようになったのですが、
なんとも泥臭い書き方で、将来項目が増えた際に対応するのが大変な予感しかありません。

理想としては、
・キーワード検索と【プール】×【勤務地】の組み合わせ検索は同一フォームだけれども別々にしたい
・組み合わせ検索で【プール】×【勤務地】のどちらかがある場合、または両方のキーワードがある場合は、それぞれのキーワードを”×”で区切って、結合した状態で変数$keywordに格納したい

の2点になります。

# index.blade.php
      <h4 class="h5 font-weight-bold mt-4">プール</h4>
      @foreach( $pools as $key => $value )
         <label class="checkbox-inline mb-1 mr-2 text-secondary">{!! Form::checkbox('pool', $key, false) !!} {{ $value }}</label>
      @endforeach

      <h4 class="h5 font-weight-bold mt-4">都道府県</h4>
      @foreach( $prefectures as $key => $value )
         <label class="checkbox-inline mb-1 mr-2 text-secondary">{!! Form::checkbox('prefecture', $key, false) !!} {{ $value }}</label>
      @endforeach

　　　<h4 class="h5 font-weight-bold mt-4">キーワード検索</h4>
      {!! Form::text('pool_name', null, ['class' => 'form-control','placeholder'=>'キーワード検索']) !!}

# controller.php
class SearchController extends Controller
{
	
	private $pools;
	private $prefectures;
	private $query;
	private $paginate_num = 2;
	
	
	public function __construct()
	{
		$this->query       = Pool::query();
		$this->prefectures = Place::$prefectures;
	}
	
	
	public function combination_search( Request $request )
	{
		$pools              = $request['pool'];
		$request_prefecture = $request['prefecture'];
		
		# ここからもっといい感じに書きたい
		if( !empty( $request['pool_name'] ) ){
			
			# プール名
			$keyword    = $request['pool_name'];
			$good_place = $this->query->where( 'pool_name', 'like', '%' . $keyword . '%' )->paginate( $this->paginate_num );
			
		} elseif( !empty( $pools || !empty( $request_prefecture ) ) ) {
			
			if( !empty( $pools && !empty( $request_prefecture ) ) ){
				
				$keyword = $this->pools[ $pools ] . " × " . $this->prefectures[ $request_prefecture ];
				
			} elseif( !empty( $pools && empty( $request_prefecture ) ) ) {
				
				$keyword = $this->pools[ $pools ];
				
			} elseif( empty( $pools && !empty( $request_prefecture ) ) ) {
				
				$keyword = $this->prefectures[ $request_prefecture ];
				
			}
			
			$params = [
				'pool',
				'prefecture',
			];
			
			foreach( $request->only( $params ) as $key => $value ){
				$this->query->where( $key, 'like', '%' . $value . '%' );
			}
			
			$good_place = $this->query->paginate( $this->paginate_num );
			
		} else {
			
			# 検索条件の指定がなかった場合=新着情報
			$keyword    = "新着";
			$good_place = Pool::paginate( $this->paginate_num );
			
		}
		
		return view( 'pool.list' )->with( [
			'good_place'     => $good_place,
			'keyword'        => $keyword,
			'pools'          => $this->pools,
			'contract_types' => $this->contract_types,
		] );
	}
	
}

環境

PHP 7.1.23
Laravel Framework 5.8.7

以上となりますが、
お忙しいところ大変恐縮ですが、ご教授いただけると幸いです。
それでは宜しくお願い致します。

Answer 1

複雑な条件分岐の場合は、複雑に書くしかありませんが、別のコントローラやコマンドで使いたくなった時のために、モデルのローカルスコープに処理を移すことが考えられます。
Laravel 5.8 Eloquent：利用の開始 ローカルスコープ

コードを短く書きたい場合は、クエリビルダーのwhenメソッドを使うとメソッドチェーンで簡潔にかけます。

$this->query->when(条件判定A,function($q){
    $q->where(追加のWhere句A)
  })->when(条件判定B,function($q){
    $q->where(追加のWhere句B)
  })->paginate();

Laravel 5.8 データベース：クエリビルダ 条件節

Answer 2

ちゃんと見てないけど、半分ぐらいモデルに持っていけるのでは？

Answer 3

やっつけですが書いてみました。
要件がわからない部分もあるので意図が変わってしまってる場所があるかもしれませんがご参考程度にどうぞ。

    public function combination_search(Request $request)
    {
        $request_pool = $request->input('pool');
        $request_prefecture = $request->input('prefecture');
        $pool_name = $request->input('pool_name');

        $keyword = $this->create_keyword($pool_name, $request_pool, $request_prefecture);
        $good_place = $this->search_good_place($pool_name, $request_pool, $request_prefecture);

        return view('pool.list')->with([
            'good_place' => $good_place,
            'keyword' => $keyword,
            'pools' => $this->pools,
            'prefectures' => $this->prefectures,
        ]);
    }

    private function create_keyword($pool_name, $request_pool, $request_prefecture)
    {
        if ($pool_name) {
            return $pool_name;
        }

        if ($request_pool && $request_prefecture) {
            return $this->pools[$request_pool] . " × " . $this->prefectures[$request_prefecture];
        }

        if ($request_pool) {
            return $this->pools[$request_pool];
        }

        if ($request_prefecture) {
            return $this->prefectures[$request_prefecture];
        }

        return '新着';
    }

    private function search_good_place($pool_name, $request_pool, $request_prefecture)
    {
        if ($pool_name) {
            return $this->query
                ->where('pool_name', 'like', '%' . $pool_name . '%')
                ->paginate($this->paginate_num);
        }

        if ($request_pool) {
            $this->query->where('pool', 'like', '%' . $request_pool . '%');
        }
        if ($request_prefecture) {
            $this->query->where('prefecture', 'like', '%' . $request_pool . '%');
        }

        return $this->query->paginate($this->paginate_num);
    }

直してみたポイントは、

• $requestからの値の受け取りは配列アクセスではなくて->input()を使う
• $requestにアクセスする場所は限定して、変数に一旦格納してから使う
• keywordを作成する部分と検索する部分を別の関数に切り出す
• 切り出した関数内ではガード節を使ってネストを浅くする

あたりです。

その他、

• poolとprefectureってcheckboxだけど複数入ってくるのを考慮しなくていいのかな？
• !empty( $pools || !empty( $request_prefecture ) ) とかの条件って !empty( $pools ) || !empty( $request_prefecture ) の間違い？
• 'contract_types' => $this->contract_types, が無かったんでprefecturesの間違い？
• $this->pools、$this->prefecturesには、IDをキーにして名前でも入っているのかと思ったんですが、検索するときの、poolとprefectureのクエリ、LIKE検索で良いのでしょうか？（IDとかで検索するのではない？）

あたり気になったのですが、適当に解釈しました。

---

将来項目が増えた際に対応するのが大変な予感しかありません。

とのことですが、私は項目が増えたときのことは増えたときに考えれば良い派です。
ですので、今回私が提示したコードは、とりあえず現状のコードの意図がわかりやすくなるように分割しただけです。

ただ、もし項目が増えたときは、ここに書き加えて対応してもいいですが、
正直なところ、Controller内にクエリが入ってくるのはあまり良い設計とは言えませんので、
別途検索サービスのような別クラスを作ったりして、コントローラーはシンプルにしたほうが保守性は上がると思います。