Refactor eloquent database query

Question

I currently query my DB based on a users input selection of 4 dropdowns. Only one input can be selected at a time. It all works fine but I'm looking at it thinking there has to be a way to refactor it rather than have a bunch of if / elseif statements.

My form looks like this

<form class="form" action="{{ route('topic-filter', $topic->id) }}" method="POST" class="topic__filters">
    @csrf
    <input type="hidden" name="division_id" value="{{ $topic->id }}">

    <div class="topic-filters__item">
        <label class="form__label" for="country">Country</label>
        <x-country-select />
    </div>

    <div class="topic-filters__item">
        <label class="form__label" for="region">Region</label>
        <x-region-select />
    </div>

    <div class="topic-filters__item">
        <label class="form__label" for="county">County</label>
        <x-county-select />
    </div>

    <div class="topic-filters__item">
        <label class="form__label" for="constituency">Constituency</label>
        <x-constituency />
    </div>
</form>

My Controller method looks like this

public function filterTopicResults(Request $request)
    {

        $request->validate([
            'division_id' => 'required|integer',
        ]);

        $division_id = $request->get('division_id');
        $country = $request->get('country');
        $county = $request->get('county');
        $region = $request->get('region');
        $constituency = $request->get('constituency');

        if ($country) {
            $votes = DB::table('mp_votes')
                ->join('mps', 'mp_votes.mp_id', '=', 'mps.id')
                ->join('topics', 'mp_votes.division_id', '=', 'topics.id')
                ->select('topics.*', 'mp_votes.mp_aye_vote', 'mp_votes.mp_noe_vote', 'mp_votes.mp_no_vote_recorded')
                ->where('topics.id', $division_id)
                ->where('mps.country', $country)
                ->get();
        } elseif ($county) {
            $votes = DB::table('mp_votes')
                ->join('mps', 'mp_votes.mp_id', '=', 'mps.id')
                ->join('topics', 'mp_votes.division_id', '=', 'topics.id')
                ->select('topics.*', 'mp_votes.mp_aye_vote', 'mp_votes.mp_noe_vote', 'mp_votes.mp_no_vote_recorded')
                ->where('topics.id', $division_id)
                ->where('mps.county', $county)
                ->get();
        } elseif ($region) {
            $votes = DB::table('mp_votes')
                ->join('mps', 'mp_votes.mp_id', '=', 'mps.id')
                ->join('topics', 'mp_votes.division_id', '=', 'topics.id')
                ->select('topics.*', 'mp_votes.mp_aye_vote', 'mp_votes.mp_noe_vote', 'mp_votes.mp_no_vote_recorded')
                ->where('topics.id', $division_id)
                ->where('mps.region', $region)
                ->get();
        } elseif ($constituency) {
            $votes = DB::table('mp_votes')
                ->join('mps', 'mp_votes.mp_id', '=', 'mps.id')
                ->join('topics', 'mp_votes.division_id', '=', 'topics.id')
                ->select('topics.*', 'mps.name', 'mp_votes.mp_aye_vote', 'mp_votes.mp_noe_vote', 'mp_votes.mp_no_vote_recorded')
                ->where('topics.id', $division_id)
                ->where('mps.constituency', $constituency)
                ->get();
        }

        $topic = Topic::withSum('userVotes', 'user_aye_count')
            ->withSum('userVotes', 'user_noe_count')
            ->where('id', $division_id)
            ->first();

        $mpVotes = [
            'total' => ($votes->where('mp_aye_vote', 1)->count() + $votes->where('mp_noe_vote', 1)->count()),
            'ayes' => $votes->where('mp_aye_vote', 1)->count(),
            'noes' => $votes->where('mp_noe_vote', 1)->count(),
            'no_vote' => $votes->where('mp_no_vote_recorded', 1)->count()
        ];

       

        return view('topics.topic', compact([
            'topic',
            'votes',
            'mpVotes',
        ]));
    }

I think you can replace the 4 blocks with 4 conditional statements — apokryfos
– apokryfos, Commented Mar 4, 2021 at 20:28
@apokryfos is it possible to use multiple ->when() in a single query? I tried but only works for the first dropdown / select. — CIB
– CIB, Commented Mar 5, 2021 at 8:38

apokryfos · Accepted Answer · 2021-03-05 17:23:29Z

1

You can try:

First validate your input to make sure you only have one filter selected
Use when

$request->validate([
   'division_id' => 'required|integer',
]);
$division_id = $request->get('division_id');
$country = $request->get('country');
$county = $request->get('county');
$region = $request->get('region');
$constituency = $request->get('constituency');
$votes = DB::table('mp_votes')
    ->join('mps', 'mp_votes.mp_id', '=', 'mps.id')
    ->join('topics', 'mp_votes.division_id', '=', 'topics.id')
    ->select('topics.*', 'mp_votes.mp_aye_vote', 'mp_votes.mp_noe_vote', 'mp_votes.mp_no_vote_recorded')
    ->where('topics.id', $division_id)
    ->when($country, function ($query, $country) { $query->where('mps.country', $country); })
    ->when($county, function ($query, $county) { $query->where('mps.county', $county); })
    ->when($region, function ($query, $region) { $query->where('mps.region', $region); })
    ->when($constituency, function ($query, $constituency) { $query->where('mps.constituency', $constituency); })
    ->get();

// ...

edited Mar 5, 2021 at 17:23

answered Mar 5, 2021 at 9:49

apokryfos

40.9k11 gold badges85 silver badges128 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

CIB Over a year ago

That's the sort of thing I'm looking for but getting Validation rule exclude_if requires at least 2 parameters

CIB Over a year ago

If I remove the validator and do $country = $request->get('country') ?? null; for each input it works

apokryfos Over a year ago

Yes I've updated that, however keep in mind there's nothing preventing a user from passing a country, county, region and constituency at the same time and getting all filters applied. Probably not a bit issue but it is different to your original code

CIB Over a year ago

The form submits on select change so that’s not an issue. I’ll check the code and accept the answer later when I’m back online

Lysander Cox · Accepted Answer · 2021-03-04 21:19:10Z

Just do this:

public function filterTopicResults(Request $request)
{

    $request->validate([
        'division_id' => 'required|integer',
    ]);

    $division_id = $request->get('division_id');
    $country = $request->get('country');
    $county = $request->get('county');
    $region = $request->get('region');
    $constituency = $request->get('constituency');

    if ($country) {
        $field = 'mps.country';
        $value = $country;
    } elseif ($county) {
        $field = 'mps.county';
        $value = $county;
    } elseif ($region) {
        $field = 'mps.region';
        $value = $region;
    } elseif ($constituency) {
        $field = 'mps.constituency';
        $value = $constituency;
    }
    $votes = DB::table('mp_votes')
        ->join('mps', 'mp_votes.mp_id', '=', 'mps.id')
        ->join('topics', 'mp_votes.division_id', '=', 'topics.id')
        ->select('topics.*', 'mp_votes.mp_aye_vote', 'mp_votes.mp_noe_vote', 'mp_votes.mp_no_vote_recorded')
        ->where('topics.id', $division_id)
        ->where($field, $value)
        ->get();

    $topic = Topic::withSum('userVotes', 'user_aye_count')
        ->withSum('userVotes', 'user_noe_count')
        ->where('id', $division_id)
        ->first();

    $mpVotes = [
        'total' => ($votes->where('mp_aye_vote', 1)->count() + $votes->where('mp_noe_vote', 1)->count()),
        'ayes' => $votes->where('mp_aye_vote', 1)->count(),
        'noes' => $votes->where('mp_noe_vote', 1)->count(),
        'no_vote' => $votes->where('mp_no_vote_recorded', 1)->count()
    ];



    return view('topics.topic', compact([
        'topic',
        'votes',
        'mpVotes',
    ]));
}

It cuts down some of the code just didn't know if it was possible to do without any if else if statements

Collectives™ on Stack Overflow

Refactor eloquent database query

2 Answers 2

4 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related