Thanks for your article. I agree with most of your refactoring except the first one. I really disagree with the idea of a method User::createFromCourseEnrollmentRequest.
1/ It's just 4 values that are usually used to create a User (first name, last name, email, password). We can assume there is nothing special related to the context of the Course Enrollment.
2/ It binds your model to a specific Request (so I can imagine to the Laravel Framework) which is IMO a very bad idea. Here it's just moving some code to hide it in the Model, it's not refactoring. Actually, in my opinion, it will lower the reusability and maintenance of your code.
What if you want to use something else than Laravel tomorrow?
For these reasons I would prefer the Before version. But I like the idea of a method to return a User (created or fetched) :).
I build things with my hands. The human behind Shift - https://laravelshift.com, master of Git - https://gettinggit.com, and author of "BaseCode" - https://basecodefieldguide.com
To be clear, there is no actual Model/Request coupling. It's just coincidence that we pass the same object. To your point, we could easily use $request->only() to pluck these values out to pass down to the model.
as for me this static method doesn't introduce a new behaviour to the user model. design pattern wise, it basically uses static factory method (i actually like it), although i preferred to have some sort of EnrollmentRegistrationService for this.
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
Thanks for your article. I agree with most of your refactoring except the first one. I really disagree with the idea of a method
User::createFromCourseEnrollmentRequest.1/ It's just 4 values that are usually used to create a User (first name, last name, email, password). We can assume there is nothing special related to the context of the Course Enrollment.
2/ It binds your model to a specific Request (so I can imagine to the Laravel Framework) which is IMO a very bad idea. Here it's just moving some code to hide it in the Model, it's not refactoring. Actually, in my opinion, it will lower the reusability and maintenance of your code.
What if you want to use something else than Laravel tomorrow?
For these reasons I would prefer the Before version. But I like the idea of a method to return a User (created or fetched) :).
To be clear, there is no actual Model/Request coupling. It's just coincidence that we pass the same object. To your point, we could easily use
$request->only()to pluck these values out to pass down to the model.as for me this static method doesn't introduce a new behaviour to the user model. design pattern wise, it basically uses static factory method (i actually like it), although i preferred to have some sort of EnrollmentRegistrationService for this.