1
\$\begingroup\$

I am still learning Laravel and I am in the process of creating a small shopping website.

On the left hand side there are a series of filters that appear depending on what products have come back from the search, to allow the user to filter their search further.

For example, if none of the products searched for return at least one product that is for "0 - 6 months old", then that filter will not appear.

I think it is a little messy, particularly the parts responsible for showing how many products match a certain filter (the little number that appears next to the filter name):

0 - 6 months old (5 products)


<h4>Product filters</h4>

    <?PHP
    $brands = $prices = $ages = $brandsUsed = $agesUsed = array();
    $out = '';
    foreach ($products as $product) {
        $brands[$product->brands->id] = $product->brands->brand;
        $brandsUsed[] = $product->brands->id;
        $prices[] = $product->price;
        $ages[$product->ages->id] = $product->ages->age;
        $agesUsed[] = $product->ages->id;
    }
    $brandsUsed = array_count_values($brandsUsed);
    $brands = array_unique($brands);
    if (count($brands) > 0) {
        $out .= '<h5>Brands</h5>';
        foreach ($brands as $brandId => $brandName) {
            $out .= '<div class="checkbox">';
            $out .= '<label>';
            $out .= '<input value="' . $brandId . '" type="checkbox" checked> ' . $brandName . ' (' . $brandsUsed[$brandId] . ')';
            $out .= '</label>';
            $out .= '</div>';
        }
    }
    sort($prices);
    $pricesOriginal = $prices;
    $prices = array_count_values($prices);
    $out .= '<h5>Price</h5>';
    $sub100 = $_100_200 = $_200_500 = $_500_1000 = $_1000_2000 = $over2000 = 0;
    foreach ($prices as $price => $quantity) {
        if ($price < 100) {
            $sub100++;
        }
        if ($price >= 100 && $price <= 200) {
            $_100_200++;
        }
        if ($price > 200 && $price <= 500) {
            $_200_500++;
        }
        if ($price > 500 && $price <= 1000) {
            $_500_1000++;
        }
        if ($price > 1000 && $price <= 2000) {
            $_1000_2000++;
        }
        if ($price > 2000) {
            $over2000++;
        }
    }
    if ($sub100 > 0) {
        $out .= '<div class="checkbox">';
        $out .= '<label>';
        $out .= '<input value="sub100" type="checkbox" checked> Under $100 (' . $sub100 . ')';
        $out .= '</label>';
        $out .= '</div>';
    }
    if ($_100_200 > 0) {
        $out .= '<div class="checkbox">';
        $out .= '<label>';
        $out .= '<input value="100-200" type="checkbox" checked> $100 - $200 (' . $_100_200 . ')';
        $out .= '</label>';
        $out .= '</div>';
    }
    if ($_200_500 > 0) {
        $out .= '<div class="checkbox">';
        $out .= '<label>';
        $out .= '<input value="200-500" type="checkbox" checked> $200 - $500 (' . $_200_500 . ')';
        $out .= '</label>';
        $out .= '</div>';
    }
    if ($_500_1000 > 0) {
        $out .= '<div class="checkbox">';
        $out .= '<label>';
        $out .= '<input value="500-1000" type="checkbox" checked> $500 - $1000 (' . $_500_1000 . ')';
        $out .= '</label>';
        $out .= '</div>';
    }
    if ($_1000_2000 > 0) {
        $out .= '<div class="checkbox">';
        $out .= '<label>';
        $out .= '<input value="1000-2000" type="checkbox" checked> $1000 - $2000 (' . $_1000_2000 . ')';
        $out .= '</label>';
        $out .= '</div>';
    }
    if ($over2000 > 0) {
        $out .= '<div class="checkbox">';
        $out .= '<label>';
        $out .= '<input value="over2000" type="checkbox" checked> Over $2000 (' . $over2000 . ')';
        $out .= '</label>';
        $out .= '</div>';
    }

    $agesCounts = array_count_values($agesUsed);
    $ages = array_unique($ages);
    foreach ($ages as $ageId => $age) {
        $out .= '<div class="checkbox">';
        $out .= '<label>';
        $out .= '<input value="' . $ageId . '" type="checkbox" checked> ' . $age . ' (' . $agesCounts[$ageId] . ')';
        $out .= '</label>';
        $out .= '</div>';
    }
    echo $out;
    ?>
\$\endgroup\$
1
\$\begingroup\$

Mutually exclusive if statements

These kind of if statements cannot be all true at the same time:

if ($price < 100) {
    $sub100++;
}
if ($price >= 100 && $price <= 200) {
    $_100_200++;
}
if ($price > 200 && $price <= 500) {
    $_200_500++;
}

Use elseif when evaluating mutually exclusive conditions, otherwise the program will unnecessarily evaluate all conditions always.

Duplicated logic

There is repetition in these if statements:

if ($_100_200 > 0) {
    $out .= '<div class="checkbox">';
    $out .= '<label>';
    $out .= '<input value="100-200" type="checkbox" checked> $100 - $200 (' . $_100_200 . ')';
    $out .= '</label>';
    $out .= '</div>';
}
if ($_200_500 > 0) {
    $out .= '<div class="checkbox">';
    $out .= '<label>';
    $out .= '<input value="200-500" type="checkbox" checked> $200 - $500 (' . $_200_500 . ')';
    $out .= '</label>';
    $out .= '</div>';
}
// ... and so on

It would be better to create a helper function that you can parameterize with the range, and simplify these and other statements:

if ($_100_200 > 0) {
    $out .= format_range(100, 200, $_100_200);
}
if ($_200_500 > 0) {
    $out .= format_range(200, 500, $_200_500);
}
// ... and so on
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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