Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.5] Add Collection::wrap method #20055

Merged
merged 1 commit into from
Jul 14, 2017

Conversation

thecrypticace
Copy link
Contributor

If the given value is not a collection, wrap it in one. This is the collection equivalent to Arr::wrap.

If you pass a collection or an array as the value you get a collection with data in the collection or array back. If you pass in anything else you get a collection of one containing that value.

Had the idea while reading the code for PR #20053. It could turn 7 LoC (at the end of setInverseRelation) in that PR to this:

Collection::wrap($models)->each->setRelation($this->inverseSide, $parent);

No more pesky instanceof check! and it reads rather well too.

public static function wrap($value)
{
return $value instanceof self
? new static($value->all())
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason not to just return $value here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. getArrayableItems correctly deals with this case. Updating…

If the given value is not a collection, wrap it in one.

If you pass a collection or an array as the value you get a collection with data in the collection or array back. If you pass in anything else you get a collection of one containing that value.
@thecrypticace thecrypticace force-pushed the feature/collection-wrap branch from 67e46a2 to f37f38e Compare July 14, 2017 01:58
@decadence
Copy link
Contributor

#19974
#19970

@taylorotwell taylorotwell merged commit f57b9e2 into laravel:master Jul 14, 2017
@taylorotwell
Copy link
Member

I guess I'll merge this because people will just keep sending it. 😬

@decadence
Copy link
Contributor

Haha Taylor can be brute forced :)

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.

4 participants