framework icon indicating copy to clipboard operation
framework copied to clipboard

[9.x] Add collect key parameter to replicate the behavior of Response::collect method

Open brenjt opened this issue 3 years ago • 2 comments

The Illuminate\Http\Client\Response::collect method makes it really easy to convert a json object to a collection object. It would be very convenient to have a similar functionality in the collection class itself however the collect method does not accept an argument.

Given this data structure

{
    "results": [1,2,3],
    "meta": [1,2,3]
}

To access the sub keys as a collection you'd need to do this:

$data = Http::get()->collect();
$results = collect($data->get('results'));
$meta = collect($data->get('meta'));

But with this PR you can simply do this:

$data = Http::get()->collect();
$results = $data->collect('results');

I recognize that you could do this

$response = Http::get();
$results = $response->collect('results');

However when used in this context it makes it not so convenient since you can't cache a response object

public function templates(): Collection
{
    return collect($this->getTemplates()->get('templates'))->mapInto(Template::class);
}

public function upsells(): Collection
{
    return collect($this->getTemplates()->get('upsells'))->mapInto(Upsell::class);
}

protected function getTemplates(): Collection
{
    return cache()->remember(
        key:'templates',
        ttl: now()->addMinutes(10),
        callback: fn () => Http::asJson()->get('endpoint')->collect()
    );
}

brenjt avatar Oct 19 '22 22:10 brenjt

I've always wished for a method that takes a key and returns that value as a collection. I think it's a great idea 👍

I'm just not so keen on overloading the existing collect() method. I think it would make more sense to come up with a name for a different method instead.


The problem with using collect() is that it's conflating two separate concerns into a single method.

The collect() method was originally added to the lazy collection to convert it into an eager collection. It was later added to the regular collection for consistency, so that when you have an instance of Enumerable you could always call collect() and be sure you have an eager collection.

Adding collect($key) to the lazy collection does not make sense IMO. Lazy collections do not have random access. Doing collect($key) on them enumerates the whole collection first, which is unexpected.

JosephSilber avatar Oct 24 '22 12:10 JosephSilber

That is a good point, I hadn't thought of the LazyCollection in this scenario. Let me give it some thought on a better name and implementation.

brenjt avatar Oct 25 '22 04:10 brenjt