Junior Code Review: Cleaning Up Laravel CRUD 

Hello good day. I am a SAP Developer with more than 10 years of experience. And I have started with laravel 1 year ago. When I'm developing in Laravel, I don't have "logic" issues, but "best practices" issues. And I get stuck trying to find the best way to do something. Your videos are helping me a lot in that matter. I find your trainings very interesting and helpful. Thank you.

All of the review are so amazing, never get bored when watching your video

This is a great series, I get so much value from it. Also, I do like the idea at the end of focusing on a specific type of refactor on each episode. That sounds like a fantastic direction to go.

Hey this is my student! In my opinion he is pro not junior :) I’m glad that I have learned him so much. Greetings from pingdevs dot com, Macedonia.

Thank you, Povilas. Your code review videos are very educational.

My New favourite channel on RU-vid 🔥🔥 especially these code reviews 🙏🏾

repetition is the mother of learning ))

Thank you for the Spatie Media Library tip!!! I've just watched their tutorial. It is amazing.

You'll probably be a man of the year in a laravel community soon

Another thing I noticed very briefly: Unless I missed another use of $staticpages in the homepage view that needed all staticpages, the loop around $staticpages was calling ->take(4) which means they were querying ALL static pages, but only needing 4. If that is the case, you should not be using a straight up all() and instead be using ->limit(4)->get() or ->take(4)->get() Getting all then only getting 4 from there is VERY expensive on the database. And databases in scaled applications are often not on the same server as the web server, meaning there's a larger load on the network now.

cool I didn't know about the spatie media library ! Thanks for that ! ^^ I will use it for one of my project !

because your videos... php storm is become my next IDE :D

Great idea, nice job.

What Laravel admin dashboard would you recommend? I've used Vuetify in the past, and I liked it. I was looking for something more lightweight, but with VueJS functionality.

Thank you!

Povilas your job is awesome! You could do a more advance code review too? A thank you from Brazil!

Quick note about the first issue, rather than having to rename the class and file name, you could just anonymise the migration class: `return new class extends Migration`. I believe in newer Laravel versions this is actually the default so my note may be a bit outdated 😅

I' m realy like your tutorial.

Hi from russia. Its very useful, especially for me. Keep it up, your doing well 👍🏻

wonderful video

On 12:40 he could use inside the function (Request $request, User $id) instead everytime to FindOrFail the user id

Thx for the video! Could you or someone please help me with the following; I have a 'catch all route' with "->where('menu', '.*')" and in my controller i'm loading in a bunch of variables though not all variables are nessecary for each single page. It depends on certains fields and/or relationships. I'm pretty sure there is a way to conditionally load in variables based on that. Could you point me in the right direction please?

nice explaination

Thanks sir

cool I didn't know about the spatie media library ! Thanks for that ! ^^ I will use it for one of my project ! I would like to know if you are able to talk also about scoping in Laravel. I've been in a company that have a senior developer, and use a loooot of scopes through the hole project. And he never uses repositories (just one or two), even so it was fully needed because of the bunch of requests he made in model. I like the repository design pattern for complexe relationships, I often use repositories (when it's necessary of course). But do you have a simple example when using scopes, when using repositories ? And by the way, could you tell also what is the difference between a trait and a helper ? ^^ Thanks for reading me, and keep going, I love to watch your videos :)

Hi! I'm enjoying all your coder reviews. Question with validation: I started to do things with LiveWire. In the docs of LiveWire "You might be wondering if you can use Laravel's "FormRequest"s. Due to the nature of Livewire, hooking into the http request wouldn't make sense. For now, this functionality is not possible or recommended." What do you think? Is more secure or has better performance do the backand things without Livewire and use FormRequest and validated type of checking the Request data or can I do with LiveWire without any issues on long term?

Hello Mr Povilas , first thank you so much for your great content , my question is do we need csrf token when using LiveWire ?

Thank you for video. 9:36 you deleted "check.user:Admin". But all users still can register and right a way have access to admin panel!!! Should to prevent other user to have registration on system or not delete "check.user:Admin" middleware ;)

8:36 its shorter way to set this variables, you may use Constructor Property Promotion (new in php 8): public function __construct() { private $settings = Settings::findOrFail(1); }

latest() orders by created_at date, not by ID. In this case it might be fine but generally it's wrong. It should be latest('id') instead.

thanks

3:57 which chrome extension is it??

0:41. I like to follow the RoR-way of naming migrations. E.g., if I want to add the field `name` to the Contact model, I will call the migration AddNameToContact. That way, there should be no collisions, and the file name very clearly describes what the migration does.

Hey can you make a video about Porto pattern (apiato) ? Thanks

Probably it's worth noticing that during development it doesn't make sens to make additional migrations just to add a new field in the table. It wouldn't make more sense to just add those fiels in the original migration and be done with it? PS I am just starting with laravel so please correct me if I am wrong.

6:48 he selects all static pages but as i understand method take he only uses 4 of them? So he should select only 4 static pages in controller, not all.

Thank u very much. Can i ask what SQL tool you use?

find(1) to First()

gg man 🔥👌🏼

hello .. what's do you use to fill de forms with data? thanks

The ‘=‘ argument in where() is not required, but occasionally not writing it may cause an error, that will be hard to debug. If search term in where will be “=“ it will cause an error of Builder, for not writing the third parameter. Of course, it will happen with input from user more likely, but, who knows.

Actually I create trait "CanUpload" in every project for file upload and use whenever and wherever I need it.

nice

Can anyone explain me that how can i set up project2.test url for my project

Great tutorial,may I send my code for review?

I liked the admin theme, What is it?

good

Awesome content, Thank you! Would be nice to have an example for the Laravel project which has both blade front end and also sanctum CRUD API access from other places. public function store(CreateJobRequest $request) { $job = $request->validated(); $job = new Job($job); $job->author()->associate($request->user()); $job->save(); if($request->expectsJson()) { return $this->success($job); } return redirect()->route('job.show', $job->slug); } right now i'm doing this but not sure it's the best way of doing it or not ?

2:53 what is the name of this mysql software?

PHPStorme theme ?

Hey! What Phpstorm theme and color scheme is this?

Call a function in array declaration is not a good idea

He can put that setting in provider using blind

Awosem content

The email address of the guy is in the address bar...

find the declaration of $data = [ ]; in each controller quite pointless when you can just do something along the lines of view('view')->with([ '' => '']) or view('view', [ '' => '' ]) It's not really proper handling of (large) view/result/response data collections anyway, but if you must really make logic like this just keep it simple and pass all finals inside the ->with() method or directly inside the view() $data parameter, instead of declaring a variable you're never going to modify in the first place. I think it's bad altogether to generate a lot of response arrays inside the controller, while usually you can segregate this into a defined object, but in some situations something along the lines of this is acceptable since it's easy to read and easy to identify it being a final/result collection of data. Also it is accepted for very generic controllers that have next to no logic to split up into an object of it's own and where you want to keep this logic inside the controller since it can invoke unnecessary complexity if you decide to OOP when you don't have to OOP. // Note don't declare a variable for result collections if it's behavior won't change $data = [ 'posts' => Post::all(), 'meta' => MetaData::render(), // ... many other things ]; // While scanning the code I'm expecting something to happen here, but nothing happened... return view('view', $data); // A little bit better.. It is short, easy to read : but keep track of the complexity of your controller and consider making a service return view('view', [ 'posts' => Post::all(), 'meta' => MetaData::render(), // Shouldn't be served from here, consider view::share() // ... many other things ]);

I'm working with some legacy code right now (procedural php that's 15 years old, not laravel) and the programmer not only put retrieved all data and put it into variables, he put them into variables named differently than the data being put in so signTypeID is put into modelID, then he checks the data and puts it into an array in an array element named model. That's then put into a form element named signID, when it's saved it's put into a json variable named modelNumber and at the receiving code it's renamed multiple more times. I have no way to know what data I'm working with at any time. That's just a single actual example, he did this with practically every pace of data pulled out of the database. I actually had one function that was over 3000 lines of spaghetti code littered with renamed data, if thens, repeated manipulations just copy/pasted instead of calls to a single function that did the work, no case switch statements where they were obviously begging for them. It's actually kind of impressive that it even worked, though it worked poorly. The sound of...let's call it frustration...coming out of my office is pretty constant. Don't do it people, for the sanity of the guy who has to come after you and fix your code.

While you are definitely providing useful information to many, I think you shouldn't be doing QA for code that hadn't been tested even once. Speaking of site settings/config, I would appreciate your take on making such a package, or if you already have one in mind, could you mention it in a reply, please. Keep up the good work. 👌🏻

is not good to do ::all(), maybe some eager loading ? optimization problems ahead ....

You should not rename migrations this way!

3:56 he/she is not validating.

It feels to me like the junior is kinda of an a-hole,trying to get you to fix his sht or just trolling to waste your time.