Using a custom collection for a model

[mikemand]

Hello,

I have a model that I use a custom collection with, to do some aggregations and such. Currently, I have to set my return typehint to \Illuminate\Database\Eloquent\Collection instead of my custom collection, because of:

laravel-jit-loader/src/Concerns/AutoloadsRelationships.php

Line 143 in 6f0091e

public function newCollection(array $models = []): Collection

Not a deal-breaker, as I can use the phpdoc to tell my IDE what the real return type is. However, with me overwriting the newCollection method, what does that do to autoloading for this model? As far as I can tell, the only important line is 145, but I don't know what it does exactly...

Are there some side-effects I am not aware of with using my own custom collection with your package?

[liam-wiltshire]

Hi @mikemand

I must confess I've not tried it with a custom collection.

The newCollection method is important, as we use that to tell each model in the collection that they are part of the collection (out of the box, a model in a Laravel collection doesn't actually have any knowledge of the collection it's a member of).

If you can send over some sample code that shows how you're trying to use it I'll have a play with it and get back to you,

[mikemand]

Hi @liam-wiltshire,

It's pretty generic, I think. Here's the code for my model:

<?

namespace App\Entities;

use App\Entities\Collections\SaleCollection;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\SoftDeletes;
use LiamWiltshire\LaravelJitLoader\Concerns\AutoloadsRelationships;

class Sale extends Model
{
	use SoftDeletes, AutoloadsRelationships;

	/**
     * @return \App\Entities\Collections\SaleCollection
     */
    public function newCollection(array $models = []): Collection
    {
        return new SaleCollection($models);
    }
}

My Sale model also has a few relationships to things like SaleItem and SaleTax. The custom Collection has methods to reduce the taxes for the collection of sales for reporting purposes, repeat for discounts on the sale items, etc. Really, you could test with static values on the Sale itself for simplicity's sake.

I should say that I am already using the $with property on the model to eager load the items and taxes, but there are a couple of auditing-related relationships (like who created the sale, not just when) that I don't autoload all the time, since I don't need them all the time. I try to remember to lazy load them in the controllers that need them, but sometimes I forget. 😆

If you have any other questions or need more clarification, just let me know. Thanks for looking into this!

[liam-wiltshire]

Assuming you are not doing anything in the collection around the load method or similar, all you'll probably need to do is update your newCollection method:

    public function newCollection(array $models = []): Collection
    {
        $collection = new SaleCollection($models);

        unset($models);
        foreach ($collection as $model) {
            $model->parentCollection = $collection;
        }
        return $collection;
    }

The important bit in there as I think I mentioned above is making sure each model in the collection knows it is part of the collection - otherwise when a relationship is called, it will not know to call it on the collection.

[mikemand]

Hi Liam,

Awesome, good to know. Thank you for taking the time to look into this for me.

I only have a few more questions for you, but these are probably more because I'm not super conscious of high-level optimization:

1. Does it decrease the JIT performance if you were to use the collection ->each method rather than a foreach?
2. The PHP Inspections plugin for PhpStorm says that the unset could probably be removed:

Not sure if this actually does help performance, or if PHPs garbage collector will take care of this without impacting said performance.

[liam-wiltshire]

Hey @mikemand,

No problem at all. In answer to your questions:

Does it decrease the JIT performance if you were to use the collection ->each method rather than a foreach?

Depending on how you are using each, it should be exactly the same. I say this because if you are doing something like $parentModel->relation()->each(...); referencing the relation as a function call then the JIT loader doesn't support that (but equally, calling it that way would ignore any eager loading you've done, so the performance would be equally poor both ways). Calling it like $parentModel->relation->each(...); will perform equally well with both.

The caveat to this is the currently the JIT loader only partially supports nested relationships (so if inside your each callback you are loading another hasMany-type relationship on child models then the performance is better than no eager loading at all, but not as good as having proper eager loading) - this was raised in #18 and I'm looking into it, but it requires tracking parent models, relations etc, so it's not exactly straightforward!

The PHP Inspections plugin for PhpStorm says that the unset could probably be removed:

Yes it almost certainly could, the gc would pick it up when the function all ends, however it doesn't have a negative impact, and I like to keep things tidy :-)

Hope this helps :-)

[mikemand]

Hi Liam,

I was more wondering about within the newCollection method, but it's good to know this caveat about nested relationships. Thank you for the answers!