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.4] Add "make" method to Eloquent Query Builder #19015

Merged
merged 1 commit into from May 1, 2017

Conversation

Projects
None yet
@calebporzio
Contributor

calebporzio commented May 1, 2017

While recording a recent podcast, @DanielCoulbourne and I discussed the benefit of having a make method for models as an alternative to the create method that does the persistence right away. @tillkruss expressed interest in the make method as well.

Simple Example:

Post::make(['title' => 'Hello World!'])->save();

Down the road we both would love to see something like the following:

Post::make(['title' => 'Hello World!'])
    ->withAuthor($author)
    ->withCompany($company)
    ->save();

We are in the process of implementing this functionality as an alternative to:

$author->posts()->save(new Post(['title' => 'Hello World', 'company_id' => $company->id]));

For now, getting the make the make method into Laravel would be a big help in situations where you find yourself doing this: new Post(....

Thanks!

@tillkruss

This comment has been minimized.

Show comment
Hide comment
@tillkruss

tillkruss May 1, 2017

Member

We have make() method on model factories, this would make it a little more uniform and Post::newModelInstance() is rather long.

Member

tillkruss commented May 1, 2017

We have make() method on model factories, this would make it a little more uniform and Post::newModelInstance() is rather long.

@taylorotwell taylorotwell merged commit 5f79b06 into laravel:5.4 May 1, 2017

2 checks passed

continuous-integration/styleci/pr The StyleCI analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mattstauffer

This comment has been minimized.

Show comment
Hide comment
@mattstauffer

mattstauffer May 1, 2017

Contributor

👌🏻

Contributor

mattstauffer commented May 1, 2017

👌🏻

* @param array $attributes
* @return \Illuminate\Database\Eloquent\Model
*/
public function make(array $attributes = [])

This comment has been minimized.

@JosephSilber

JosephSilber May 1, 2017

Contributor

Why isn't this static?

@JosephSilber

JosephSilber May 1, 2017

Contributor

Why isn't this static?

This comment has been minimized.

@DanielCoulbourne

DanielCoulbourne May 1, 2017

Contributor

I was just having this conversation with @calebporzio because I was accustomed to seeing create() as a static method on Model.php but it has now been moved to a dynamic method on the Eloquent Builder.

Looks like Model's __callStatic() method, which this is hitting, will call it dynamically.

I may be misunderstanding, but it seems safe to me.

@DanielCoulbourne

DanielCoulbourne May 1, 2017

Contributor

I was just having this conversation with @calebporzio because I was accustomed to seeing create() as a static method on Model.php but it has now been moved to a dynamic method on the Eloquent Builder.

Looks like Model's __callStatic() method, which this is hitting, will call it dynamically.

I may be misunderstanding, but it seems safe to me.

This comment has been minimized.

@michaeldyrynda

michaeldyrynda May 1, 2017

Contributor

Unless I'm mistaken, __callStatic only works for inaccessible methods, i.e. private/protected and non-existent methods so to leverage that, you'd likely want to make the method protected if going down that path.

Otherwise, yes, as @JosephSilber suggested, it ought to be static. I'm not even certain how it's working as is 🔮

@michaeldyrynda

michaeldyrynda May 1, 2017

Contributor

Unless I'm mistaken, __callStatic only works for inaccessible methods, i.e. private/protected and non-existent methods so to leverage that, you'd likely want to make the method protected if going down that path.

Otherwise, yes, as @JosephSilber suggested, it ought to be static. I'm not even certain how it's working as is 🔮

This comment has been minimized.

@DanielCoulbourne

DanielCoulbourne May 1, 2017

Contributor

It works when called from the Model context because make() (and create(), which is also not static) are non-existent on Model.php, triggering __callStatic() which then does:
return (new static)->$method(...$parameters); calling the dynamic method on Eloquent Builder dynamically. This appears to be indended functionality based on this comment

At least that's how it seems, but I'm kind of a source-diving n00b so I could be reading this wrong.

@DanielCoulbourne

DanielCoulbourne May 1, 2017

Contributor

It works when called from the Model context because make() (and create(), which is also not static) are non-existent on Model.php, triggering __callStatic() which then does:
return (new static)->$method(...$parameters); calling the dynamic method on Eloquent Builder dynamically. This appears to be indended functionality based on this comment

At least that's how it seems, but I'm kind of a source-diving n00b so I could be reading this wrong.

This comment has been minimized.

@michaeldyrynda

michaeldyrynda May 1, 2017

Contributor

Huh, TIL. I didn't know that __callStatic worked up the tree like that 👍

@michaeldyrynda

michaeldyrynda May 1, 2017

Contributor

Huh, TIL. I didn't know that __callStatic worked up the tree like that 👍

This comment has been minimized.

@JosephSilber

JosephSilber May 1, 2017

Contributor

Oh. Looked too quickly. Thought this was defined on the model.

Of course, since it's on the builder it should not be static.

However, why is it on the builder in the first place?

@JosephSilber

JosephSilber May 1, 2017

Contributor

Oh. Looked too quickly. Thought this was defined on the model.

Of course, since it's on the builder it should not be static.

However, why is it on the builder in the first place?

@antonkomarev

This comment has been minimized.

Show comment
Hide comment
@antonkomarev

antonkomarev May 1, 2017

Contributor

Hah... just thought to write the same implementation today! Thanks!

Contributor

antonkomarev commented May 1, 2017

Hah... just thought to write the same implementation today! Thanks!

@meness

This comment has been minimized.

Show comment
Hide comment
@meness

meness May 9, 2017

Contributor

make() method released with the latest version, is it possible to do like this right now?

Post::make(['title' => 'Hello World!'])
    ->withAuthor($author)
    ->withCompany($company)
    ->save();
Contributor

meness commented May 9, 2017

make() method released with the latest version, is it possible to do like this right now?

Post::make(['title' => 'Hello World!'])
    ->withAuthor($author)
    ->withCompany($company)
    ->save();
@decadence

This comment has been minimized.

Show comment
Hide comment
@decadence

decadence May 9, 2017

Contributor

@meness No. It was just future wish. This PR adds only one method.

Contributor

decadence commented May 9, 2017

@meness No. It was just future wish. This PR adds only one method.

@MasterRO94

This comment has been minimized.

Show comment
Hide comment
@MasterRO94

MasterRO94 May 11, 2017

Contributor

What difference is between make() and fill() methods?

Contributor

MasterRO94 commented May 11, 2017

What difference is between make() and fill() methods?

@calebporzio

This comment has been minimized.

Show comment
Hide comment
@calebporzio

calebporzio May 11, 2017

Contributor

@MasterRO94 - fill() can't be called statically. makes purpose is to initialize a model by using it as a static method: Post::make() - make sense?

Contributor

calebporzio commented May 11, 2017

@MasterRO94 - fill() can't be called statically. makes purpose is to initialize a model by using it as a static method: Post::make() - make sense?

@devcircus

This comment has been minimized.

Show comment
Hide comment
@devcircus

devcircus May 12, 2017

Contributor

'Fill' uses an existing instance. 'Make' initializes a new instance and fills it with the provided data.

Contributor

devcircus commented May 12, 2017

'Fill' uses an existing instance. 'Make' initializes a new instance and fills it with the provided data.

@orrd

This comment has been minimized.

Show comment
Hide comment
@orrd

orrd May 14, 2017

Rather than this:

Post::make(['title' => 'Hello World!'])->save();

We could already just do this right:

(new Post(['title' => 'Hello World!']))->save();

I guess the make() way is just an [arguably] cleaner alternative syntax?

orrd commented May 14, 2017

Rather than this:

Post::make(['title' => 'Hello World!'])->save();

We could already just do this right:

(new Post(['title' => 'Hello World!']))->save();

I guess the make() way is just an [arguably] cleaner alternative syntax?

@calebporzio

This comment has been minimized.

Show comment
Hide comment
@calebporzio

calebporzio May 15, 2017

Contributor

@orrd - exactly

Contributor

calebporzio commented May 15, 2017

@orrd - exactly

@calebporzio

This comment has been minimized.

Show comment
Hide comment
@calebporzio

calebporzio May 15, 2017

Contributor

I just put it on the builder because that's where create lived. And it made sense to me to keep them both in the same place, especially since the only difference in implementation is the ->save() method.

Contributor

calebporzio commented May 15, 2017

I just put it on the builder because that's where create lived. And it made sense to me to keep them both in the same place, especially since the only difference in implementation is the ->save() method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment