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.
Nice article. Good that you're pasting examples. You clean controllers code but in my opinion for the big picture you should add code snippet for places where the code is moved to. I think that could be very useful otherwise it looks like you're moving code in other places.
Examples with methods refactoring suit me.
Don't give up with refactoring. Fingers crossed 😁
IMO, it's better to mirror the use of the collection method inside Set::contains() instead of using in_array, since items seems to be a Collection object. You get more functionality that way and it's more Laravel-ish.
I see your suggestion. However, it's important to remember an array is a collection. No need to introduce an entire framework just to use one method.
Ah, I see that the other calls are to the Set class size and contains. I misread - I thought it was calling those on items. My bad. You are correct.
Very interesting, I'll keep on reading your posts, thanks! This is actually one of the area I want to improve these days so your posts come in handy. Just one detail: you should make your screenshot bigger, because they're really hard to read like this, even full-size.
Unfortunately, as commented, this is a limitation of dev.to and their image upload functionality. View the original tweets for a better resolution.
Oh I'm new to dev.to and didn't know that, sorrry for the useless comment. I'll follow up on twitter as well, thanks for the heads up!
Too bad the screenshots are way to small, they seem really well edited.
This is a limitation of dev.to. View my original tweets for a higher res image.
1) To do clean code, you must stop using php
2) Models should not implement every possible way of instanciating them
3) Transfer from one account to another without transaction? This should be in a service.
4) The basic minimum for clean code is to have tests for them. I don't see any
5) No comments?
I stopped reading your reply after your first point… Clean code is not exclusive to any one language.
Either your first point is a troll, either it discredits you as a good developer.
For me it looks like a great introduction into usage of Boy Scout Rule.
Boy Scout Rule
That's not mine, but read more maybe here
Indeed, I've written about this many times.
Excellent article, problem with screencaptures is the (automated?) generated cloudinary image of 880px max width; a lightbox would be great (the zoom mouse cursor tricks us to think that the image file will open in a modal window) or a link to original image would be highly appreciated instead of manual editing the link. What I did was to convert res.cloudinary.com/practicaldev/im... into thepracticaldev.s3.amazonaws.com/i... and similarly in every image in order to watch your nice programming standards.
Who can we contact to express these drawbacks of dev.to?
Hi, great article on such a simple concept!
It seems that the $request argument is missing on line 7.
You are absolutely correct. Unfortunately it's an old screenshot and I'm too lazy to redo it. :)
I hate to sound stupid but these lines with comment blocks, I can only assume you are generating these? Using what? Or did you only manually do them for the sake of example?
Yes, I added them to a screenshot of the code for the sake of example.
Awesome article and thanks for sharing it.
For sure! Glad you enjoyed it. Follow me on Twitter to see more next week.
I was actually looking for Open Source code to write some refactoring articles, but I couldn't found one :/
I will share your work with my cadets.
Awesome. Be sure to share back once you write something up!
The screenshots are a little bit hard to see (too small) :(
Will try later from my phone xD
This is a really awesome blog. I love this blog. I wanna translate it in Bangla & post in my organization website. Would you like to give me permission to translate & repost it?
Sure. Send me the link when you're done.
Thank you very much. I will create an account for you. Then I will send you everything. Give me your email address.
Can you give me your email address for opening an account of you? I will give the post Author you.
Ah, great article.
Nice tips for cleaning your code and it is readable with comment on each line.
Thanks for your article, Jason.
IMO clean code is not just about architectures... a code is also cleaner when it can communicate it's intention without the use of comments. like example one
Thanks for the wonderful tips.
Really, the only complaint I have in this post is the screenshots are too small. I barely able to read them.
What's the code editor on screenshots? Vim or atom?
Atom with the default theme.
Thanks Jason. These tips really helped me.
Great. Check out BaseCode for even more practices.
We're a place where coders share, stay up-to-date and grow their careers.
We strive for transparency and don't collect excess data.