re: A month of clean code VIEW POST

VIEW FULL DISCUSSION
 

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) :).

 

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.

 

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.

code of conduct - report abuse