-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
Conversation
We have |
👌🏻 |
* @param array $attributes | ||
* @return \Illuminate\Database\Eloquent\Model | ||
*/ | ||
public function make(array $attributes = []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't this static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 🔮
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, TIL. I didn't know that __callStatic
worked up the tree like that 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
Hah... just thought to write the same implementation today! Thanks! |
Post::make(['title' => 'Hello World!'])
->withAuthor($author)
->withCompany($company)
->save(); |
@meness No. It was just future wish. This PR adds only one method. |
What difference is between |
@MasterRO94 - |
'Fill' uses an existing instance. 'Make' initializes a new instance and fills it with the provided data. |
Rather than this:
We could already just do this right:
I guess the |
@orrd - exactly |
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 |
While recording a recent podcast, @DanielCoulbourne and I discussed the benefit of having a
make
method for models as an alternative to thecreate
method that does the persistence right away. @tillkruss expressed interest in themake
method as well.Simple Example:
Down the road we both would love to see something like the following:
We are in the process of implementing this functionality as an alternative to:
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!