CIB
CIB

Reputation: 785

Refactor eloquent database query

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',
        ]));
    }

Upvotes: 0

Views: 62

Answers (2)

apokryfos
apokryfos

Reputation: 40683

You can try:

  1. First validate your input to make sure you only have one filter selected
  2. 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();

// ...

Upvotes: 1

Lysander Cox
Lysander Cox

Reputation: 295

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',
    ]));
}

Upvotes: 0

Related Questions