Laravel 5.1 - 过滤集合不起作用

Question

I have the following two collections:

$credits = collect([
   ['quantity' => 3, 'product_id' => 1],
   ['quantity' => 2, 'product_id' => 5]
]);

$basketItems = collect([
   ['id' => 1, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 2, 'basket_id' => 4, 'product_id' => 2],
   ['id' => 3, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 4, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 5, 'basket_id' => 4, 'product_id' => 1]
]);

The $credits collection tells us the user has 3 available credits to use for product_id 1.

Now I want to create two new collections. If a user has basket items for which they have available credits - this can be determined by the product_id, then I want to add those items into a new collection called $basketItemsUseCredits.

If the basket items dont have available credits for the type of product, I want to add those items into another collection called $basketItemsPay.

So in the example above I should end up with $basketItemsUseCredits with basketitems that have id's 1,3 and 4. $basketItemsPay should end up with basketsitems that have id's 2 and 5. The following doesn't work.

  $basketItemsUseCredits = $basketItems->map(function ($basketItem) use ($credits) {

        $qty = $credits->where('product_id', $basketItem->product_id)->get('quantity', 0);

        if ($qty > 0) {
             // update quantity
             $credits->transform(function ($credit) use ($basketItem) {
                    if ($credit->product_id == $basketItem->product_id) {
                         $credit->quantity = $credit->quantity - 1;
                         return $credit;
                    }
                    else
                        return $credit
            });               

           return $basketItem;
        }
  })


   $basketItemsPay = $basketItems->map(function ($basketItem) use ($basketItemsUseCredits) {

        if (!$basketItemsUseCredits->contains('id', $basketItem->id))
              return $basketItem;
    });

    if(!$basketItemsPay->isEmpty()) {
        // do some extra stuff
    }

The following line is always returning 0:

$qty = $credits->where('product_id', $basketItem->product_id)->get('quantity', 0);

Also another thing I've noticed. If $basketItemsPay is empty e.g. if I dd($basketItemsPay) I get the following:

Collection {#316 ▼
    #items: array:1 [▼
       0 => null
    ]
 }

So why does the following always evaluate to true in the above scenario?

if(!$basketItemsPay->isEmpty()) {
        // do some extra stuff
}

Any help appreciated.

* UPDATE *

Managed to fix by doing the following - unless any one know a better solution:

$qty = $credits->where('product_id', $basketItem->product_id)->first()['quantity'];

And chaining the reject method as follows to get rid of empty values - does any one know of a more elegant solution?

$basketItemsPay = $basketItems->map(function ($basketItem) use ($basketItemsUseCredits) {

      if (!$basketItemsUseCredits->contains('id', $basketItem->id))
             return $basketItem;

})->reject(function ($basketItem) {
        return empty($basketItem);
});

Answer 1

Took me a while as well to figure this out. Below is how I would approach this. It's a slightly more linear approach, and a bit more self-explanatory than nesting closures in the Collection methods. It also is a bit more code than your example, but that's a matter of personal preference. I tried to be as descriptive as possible in the code comments.

The most important part in making things easier for ourselves lies in transforming the structure of the $credits collection at the very start:

// First, let's simplify the $credits structure for easy checking. NOTE: this collection WILL mutate by the code below.
// If the original version is to be preserved for whatever reason, now's the time to duplicate it :)
$credits = collect([
   ['quantity' => 3, 'product_id' => 1],
   ['quantity' => 2, 'product_id' => 5]
])
    ->pluck('quantity', 'product_id')

/* Results in a collection where keys are product_id, and values are quantity:

    Illuminate\Support\Collection {
        all: [
            1 => 3,
            5 => 2
        ]
    }

*/

$basketItems = collect([
   ['id' => 1, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 2, 'basket_id' => 4, 'product_id' => 2],
   ['id' => 3, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 4, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 5, 'basket_id' => 4, 'product_id' => 1]
]);


$basketItemsUseCredits  = new \Illuminate\Support\Collection;

// First, we will filter the $basketItems collection by the condition that the product_id for each item
// does NOT occur as key inside $credits. NOTE: filter() returns a new collection. The original $basketItems
// does not mutate because of this call.
$basketItemsPay = $basketItems->reject(function ($basketItem) use ($credits) {

    // If credits has a key corresponding the product_id of the current $basketItem, REJECT the $basketItem
    return $credits->has($basketItem['product_id']);
});



// Now, we will create an intermediate collection of basket items that need to be compared against the quantity
// of credits available for products of the ID's present in this collection:
$basketItemsCountCredits = $basketItems->filter(function ($basketItem) use ($credits) {

    // If credits has a key corresponding the product_id of the current $basketItem, KEEP the $basketItem
    return $credits->has($basketItem['product_id']);
});

// Lastly we will iterate the $basketItemsCountCredits collection, and determine whether credits are still available
// for each item. If so, push the item to $basketItemsUseCredits AND decrement the amount of available credits for 
// the item's ID. otherwise just push the item to $basketItemsPay.
foreach ($basketItemsCountCredits as $item) {

    $productId = $item['product_id'];
    $remainingCredits = $credits->get($productId);

    // If more than 0 credits are available for products with ID of $item['product_id']
    if ($remainingCredits > 0) {

        // .. push the $item to $basketItemsUseCredits,
        $basketItemsUseCredits->push($item);

        // .. and decrement the amount of available credits. 
        // Collection->put() overwrites the key => value pair in the collection if the key already exists.
        $credits->put($productId, $remainingCredits - 1);
    } else {

        // The amount of available credits might have become 0 in previous iterations of this for-loop
        $basketItemsPay->push($item);
    }
}

There are a lot of pretty powerful methods available on Illuminate Collections. When used in the right way, they can allow for some very clean and concise code!

But these same methods can also set you back with regard to code-readability and -complexity when used where not necessary. The methods where you'd pass a callback function are in some cases mainly useful when the definition of that callback is done elsewhere, and it relies on that context to do its job. From your example, that does not seem to be the case here.

Instead of using the filter and reject methods consecutively, like I did in the above example, you could use the good old foreach loop to perform both actions at once. This does require however that the $basketItemsCountCredits and $basketItemsPay variables are initialized as new collections in advance. If we also strip the comments, it's actually not that much code, and still perfectly readable :)

$credits = collect([
   ['quantity' => 3, 'product_id' => 1],
   ['quantity' => 2, 'product_id' => 5]
])->pluck('quantity', 'product_id')

$basketItems = collect([
   ['id' => 1, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 2, 'basket_id' => 4, 'product_id' => 2],
   ['id' => 3, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 4, 'basket_id' => 4, 'product_id' => 1],
   ['id' => 5, 'basket_id' => 4, 'product_id' => 1]
]);


$basketItemsCountCredits = new \Illuminate\Support\Collection;
$basketItemsUseCredits   = new \Illuminate\Support\Collection;
$basketItemsPay          = new \Illuminate\Support\Collection;

// A foreach loop is perfectly sane here
foreach ($basketItems as $item) {
   if ($credits->has($item['product_id'])) {
       $basketItemsCountCredits->push($item);
   } else {
       $basketItemsPay->push($item);
   }
});

foreach ($basketItemsCountCredits as $item) {

    $productId = $item['product_id'];
    $remainingCredits = $credits->get($productId);

    if ($remainingCredits > 0) {
        $basketItemsUseCredits->push($item);
        $credits->put($productId, $remainingCredits - 1);
    } else {
        $basketItemsPay->push($item);
    }
}