4
\$\begingroup\$

I have a filter component that lets a user filter for events on a website. The user needs to see which category is currently selected in the filter. For this there is a URL with parameters.

e.g. /events?category=1

The code will match the category to the <option> in the <select>.

The code:

<?php

$selected = 'selected';

if (Request::is('courses')) {
    $category = '0';
    if (request()->get('category')) {
        [$selected, $category] = match (request()->get('category')) {
            '0' => [$selected, '0'],
            '1' => [$selected, '1'],
            '2' => [$selected, '2'],
            '3' => [$selected, '3'],
            '4' => [$selected, '4'],
            '5' => [$selected, '5'],
            '6' => [$selected, '6'],
            '7' => [$selected, '7'],
            '8' => [$selected, '8'],
        };
    }
}

if (Request::is('events')) {
    $category = '1';
    if (request()->get('category')) {
        [$selected, $category] = match (request()->get('category')) {
            '0' => [$selected, '0'],
            '1' => [$selected, '1'],
            '2' => [$selected, '2'],
            '3' => [$selected, '3'],
            '4' => [$selected, '4'],
            '5' => [$selected, '5'],
            '6' => [$selected, '6'],
            '7' => [$selected, '7'],
            '8' => [$selected, '8'],
        };
    }
}

?>

<form class="container">
    <div class="bg-white p-6 rounded-lg">
        <div class="grid grid-cols-1 md:grid-cols-2 gap-6">
            <div>
                <label class="mb-3 inline-block">Category</label>
                <select  aria-label="Category" name="category"  class="w-full bg-gray-200 p-3 rounded-lg">
                    <option value="0" @if ($category === '0') Selected @endif>All</option>
                    <option value="1" @if ($category === '1') Selected @endif>Selected Events 1</option>
                    <option value="2" @if ($category === '2') Selected @endif>Selected Events 2</option>
                    <option value="3" @if ($category === '3') Selected @endif>Selected Events 3</option>
                    <option value="4" @if ($category === '4') Selected @endif>Selected Events 4</option>
                    <option value="5" @if ($category === '5') Selected @endif>Selected Events 5</option>
                    <option value="6" @if ($category === '6') Selected @endif>Selected Events 6</option>
                    <option value="7" @if ($category === '7') Selected @endif>Selected Events 7</option>
                    <option value="8" @if ($category === '8') Selected @endif>Selected Events 8</option>
                </select>
            </div>
            <div class="flex items-end">
                <x-button type="submit" class="w-full">Search</x-button>
            </div>
        </div>
    </div>
</form>

is there any way to simplify this code? I could only think of this way, but I am still a beginner with PHP. The Categories are not stored in a DB table, so getting it from the DB is not possible.

\$\endgroup\$
1

1 Answer 1

6
\$\begingroup\$

Some variable assignments are excessive

Let's look at what $selected is set to:

$selected = 'selected';

Then in both of the match statements $selected is always assigned to $selected so the return expressions from the match statements can simply return the value for $category - i.e.

    [$selected, $category] = match (request()->get('category')) {
        '0' => [$selected, '0'],
        '1' => [$selected, '1'],
        '2' => [$selected, '2'],
        '3' => [$selected, '3'],
        '4' => [$selected, '4'],
        '5' => [$selected, '5'],
        '6' => [$selected, '6'],
        '7' => [$selected, '7'],
        '8' => [$selected, '8'],
    };

can be simplified to the following:

    $category = match (request()->get('category')) {
        '0' => '0',
        '1' => '1',
        '2' => '2',
        '3' => '3',
        '4' => '4',
        '5' => '5',
        '6' => '6',
        '7' => '7',
        '8' => '8',
    };

Simplify assigning category variable

A simpler way of assigning $category would be to check if request()->get('category') has an integer value between 0 and 8. That could be achieved using intval(), in_array() and range()

$requestCategory = request()->get('category');
if (in_array(intval($requestCategory), range(0, 8))) {
    $category = $requestCategory
}

Set category default before checking request value

Removing the match statements above could lead to these first two conditions:

if (Request::is('courses')) {
    $category = '0';
} else if (Request::is('events')) {
    $category = '1';
}

Then the isset() function can be used to check if either of those conditions were true and that variable was set:

if (isset($category)) {
    $requestCategory = request()->get('category');
    if (in_array(intval($requestCategory), range(0, 8))) {
        $category = $requestCategory;
    }
}

Edit:

I saw in your suggested edit that you proposed simply checking if there is a value in request()->get('category') instead of using isset($category) - while it would technically be different functionality than the original code, that could be done instead.

In that case $requestCategory could be assigned within the conditional statement, though some argue it is less readable. I will leave it up to you to decide whether to use that or not.

if ($requestCategory = request()->get('category')) {
    if (in_array(intval($requestCategory), range(0, 8))) {
        $category = $requestCategory;
    }
}

With the assignment moved out of the conditional it could allow for the two conditionals to be combined, allowing for the indentation levels to be decreased:

$requestCategory = request()->get('category')
if ($requestCategory && in_array(intval($requestCategory), range(0, 8))) {
    $category = $requestCategory;
}

The template can have a loop

Blade templates support loops. So these repeated lines:

<option value="0" @if ($category === '0') Selected @endif>All</option>
<option value="1" @if ($category === '1') Selected @endif>Selected Events 1</option>
<option value="2" @if ($category === '2') Selected @endif>Selected Events 2</option>
<option value="3" @if ($category === '3') Selected @endif>Selected Events 3</option>
<option value="4" @if ($category === '4') Selected @endif>Selected Events 4</option>
<option value="5" @if ($category === '5') Selected @endif>Selected Events 5</option>
<option value="6" @if ($category === '6') Selected @endif>Selected Events 6</option>
<option value="7" @if ($category === '7') Selected @endif>Selected Events 7</option>
<option value="8" @if ($category === '8') Selected @endif>Selected Events 8</option>

Could be simplified to the following. Note it uses the @selected directive mentioned in the Additional Attributes section. This allows for $selected to be eliminated.

<option value="0" @selected($category === '0')>All</option>
@for ($i = 1; $i < 9; $i++)
    <option value="{{ $i }}" @selected($category == $i)>Selected Events {{ $i }}</option>
@endfor

The range() function could also be used with a @foreach loop:

<option value="0" @selected($category === '0')>All</option>
@foreach (range(1, 8) as $i)
    <option value="{{ $i }}" @selected($category == $i)>Selected Events {{ $i }}</option>
@endfor
\$\endgroup\$
2
  • \$\begingroup\$ Thank you very much, this solves most of the issues. The only thing with the loop is that the <option> dont have the same names. So it is not always "Selected Events", but "Event Name 1", "Lorem Impsum" etc. \$\endgroup\$
    – Maximilian
    Dec 14, 2022 at 10:05
  • 1
    \$\begingroup\$ @Maximilian Instead of a range function, you can use an array with all the names. For example: $categories = ['All', 'Category 1', 'Never gonna give you up', 'Lorem ipsum dolor']; in the PHP part. Then the loop can be @foreach ($categories as $i => $name) <option value="{{ $i }}" @selected($category == $i)>{{ $name }}</option> @endfor. \$\endgroup\$ Dec 14, 2022 at 12:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.