Skip to content

Conversation

dwightwatson
Copy link
Contributor

The diff and intersect functions on Illuminate\Support\Collection use native PHP functions while merge, diff and intersect functions on `Illuminate\Eloquent\Collection' work by using the primary key of provided models.

Tests included.

@taylorotwell
Copy link
Member

Why does it not use the built in PHP array diff, intersect, stuff?

@dwightwatson
Copy link
Contributor Author

The built-in functions look for identical objects where the same model may not always be so, so these use the primary key of the models to determine uniqueness.

@taylorotwell
Copy link
Member

OK my personal thinking for how this should wrok would be to name them just "diff", 'Intersect", etc. and move a base version of them down intos upport collection, which just uses the PHP native functions, then override those functions in Eloquent with what you ahve here for key checking.

@dwightwatson
Copy link
Contributor Author

Alright, I'll implement that in the pull request. Thanks!

*/
public function diff($items)
{
if ($items instanceof Collection)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is now duplicated over three methods, perhaps it could be extracted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this. Maybe a protected getArrayableItems method on Illuminate\Support\Collection?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.

@taylorotwell taylorotwell merged commit e7598f4 into laravel:master Nov 26, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants