Loop through products and return first product that matches criteria

Question

I am writing a function which loops through an array of products. The function needs to return:

• the ID of the first product that has an image and is in stock
• if no products have an image and are in stock, then return the first product id that has an image
• if no products have an image, then return the first product id that is in stock
• if no products have an image, and no products are in stock, return the first product id

This is what I have written, and it is working as expected:

function loopThroughProducts($products)
{
    $firstId = $products[0]->id;

    foreach ($products as $product) {
        if ($product->hasImage && $product->inStock) {
            $id = $product->id;
            return $id;
        }
        if ($product->hasImage && empty($firstIdWithImage)) {
            $firstIdWithImage = $product->id;
        }
        if ($product->inStock && empty($firstIdInStock)) {
            $firstIdInStock = $product->id;
        }
    }

    if (!empty($firstIdWithImage)) {
        return $firstIdWithImage;
    }
    if (!empty($firstIdInStock)) {
        return $firstIdInStock;
    }

    return $firstId;
}

Does anyone see a way that I can optimize this loop or simplify the logic? I dislike using so many sequential if-statements in one function, and there are some minor violations to the DRY principle.

Answer 1

Clean, readable code. But why not use more of PHP's standard array functions? Use the array_search or in_array functions. Or -even more fancy- the array_filter function.

Answer 2

• Yes, you can certainly nest your checks a bit better to improve DRYness.
• isset() has an advantage over empty() in this scenario because isset() accepts multiple parameters versus empty() only permits a single parameter.
• using a chain of null coalescing operators will reduce the lines of code near the end of your custom function. Here is a reference to this technique.

For anyone who got lost in your explanation of the logic:

1. Has Image, Has Stock -- use as soon as found
2. Has Image, No Stock -- use if no #1 is found in whole array
3. No Image, Has Stock -- use if no #2 is found in whole array
4. No Image, No Stock -- use default if no #3 is found in whole array

Logically, there is no reason to store an id that qualifies as #3 if a #2 id is already found.

Code: (not tested because no test data provided)

function loopThroughProducts($products) {
    foreach ($products as $product) {
        if ($product->hasImage) {
            if ($product->inStock) {
                return $product->id;
            } elseif (!isset($hasImageNoStock)) {
                $hasImageNoStock = $product->id;
            }
        } elseif ($product->inStock && !isset($hasImageNoStock, $hasStockNoImage)) {
            $hasStockNoImage = $product->id;
        }
    }
    return $hasImageNoStock ?? $hasStockNoImage ?? $products[0]->id;
}