Laravel Junior Code Review: 12 Tips on Everything

Подписаться 141 тыс.

Просмотров 74 тыс.

50% 1

After quite a while, I'm making another junior Laravel code review, with tips on migrations, routing, PSR standards, Route Model Binding, and more.
00:00 Intro
00:40 Make Migrations Work
02:59 Seed Admin User
04:48 Short Project Demo
05:16 Routes: Laravel 7/8 Way
06:24 Method Names: PSR-1
07:01 Extra Routes Before Resources
07:35 Separate Admin Controllers
08:52 Likes/Unlikes - Package?
10:01 Route Model Binding with Slugs
11:29 Middleware in Routes
12:22 Weird protected $recipe
13:43 DRY Controller Code
Related links:
- PSR-1: Basic Coding Standard www.php-fig.org/psr/psr-1/
- Laravel Love: Likes and Dislikes for Eloquent Models: laravel-news.com/laravel-love
- 4 Ways to Auto-Fill Field Value in Eloquent • 4 Ways to Auto-Fill Fi...
- - - - -
Support the channel by checking out our products:
- Enroll in my Laravel courses: laraveldaily.teachable.com
- Try our Laravel QuickAdminPanel: bit.ly/quickadminpanel
- Purchase my Livewire Kit: livewirekit.com
- View Laravel Code Examples: laravelexamples.com
- Subscribe to my weekly newsletter: bit.ly/laravel-newsletter

Хобби

Опубликовано:

13 июл 2024

Ссылка:

Скачать:

Готовим ссылку...

Добавить в:

Мой плейлист

Посмотреть позже

Комментарии : 94

@marekjurica9098 2 года назад

Thank you for review. I have to apologize for the code was not working, (migrations issue) and the readme files. I didn't mean any disrespect. I forgot on these important things. My fault.

@guilhermemoraes4055 2 года назад

Is your code anywhere in GitHub? I'd like to clone it

@guilhermemoraes4055 2 года назад

In my opinion, this kind of video are the best for junior level developers, like me, to learn from others mistakes. Thank you.

@abusayeedshakil9774 2 года назад

Please make more of these code reviews because it offers many things to learn at once especially how we should do things. Thank you.

@RasceGSalasG 2 года назад

I love the code reviews , I always learn a lot , thank you

@MuhammadHaseeb4 2 года назад

After watching all the review code series I feel like i am a junior developer even with few years experience :D. Its more effective way to learn indeed. Thank you for making these efforts. appreciate that.

@brandon_ab123 2 года назад

Please keep doing the junior code reviews. Helps a ton!

@adityakadam2256 2 года назад

Nice video. Just one suggestion. For the DRY Controller Code, though the code is becoming shorter it adds the double query to the database. May be it can be made efficient by using an array and then in the if condition adding the image key-value pair in the array and at the end create statement can come. Something like public function store(RecipeStoreRequest $request) { $recipeArray = [ 'title' => $request->title, 'category_id' => $request->category /* other key=>value pairs */ ]; if($request->hasFile('image')) { /*code for file upload*/ $recipeArray['image'] = $name; } $recipe = auth()->user()->recipes()->create($recipeArray); }

@LaravelDaily 2 года назад

Yes, I agree, good catch.

@merdmann Год назад

These videos are great because it helps enforce standards in coding and shows new devs the standards that they themselves may not be aware of

@binodthakur696 2 года назад

We learners are really feeling blessed to learn daily from you - god bless you.By your laravel tips,series and junior code review i have learned and still being exited watches your every video.

@joshuafomubod8216 2 года назад

This is great, I love this kind off video to see how others think or approach things help me see things from different angle

@ljubicasamardzic7819 2 года назад

Thank you! These are extremely helpful. Looking forward to future code reviews.

@kareemlateefyomi8690 2 года назад

Please make more of this code reviews. Learnt a lot in just one video. Thank you

@armentorosyan8268 2 года назад

Thanks for the video! Feeling myself much more confident as a Laravel beginner!

@geneartista9714 2 года назад

Truly helpful indeed, such lesson is what I need :) Thanks for your hard work.

@exikoz 2 года назад

Thank you for taking your time and teaching us this valuable information.

@fandinurrokhim2615 8 месяцев назад

thank you, as a junior developer this video really help me how to code better

@ayushmukherjee23 2 года назад

This looks more like a person who just started laravel vs even a junior. Good review!

@benouattara6249 2 года назад

what is you definition of junior ?

@VitaliyHAN 2 года назад

Thanks. Plz make more like this. Code review is good place to learn. BTW Im from Russia, I think you have the best laravel RU-vid channel in the world!

@dhellly 2 года назад

Thank you Povilas... Great Review!!!

@larrylague1359 Год назад

we learn a lot please continue making more video like this in the future

@gtsmeg3474 2 года назад

Nice video, I'm currently having multiple projects using Laravel, I currently use Laravel module package to modulate my application

@targetx1733 2 года назад

You could atleast two reviews per week. It doesn't only helpful for junior developers,but it helps keeps more advanced devs in line and current

@LaravelDaily 2 года назад

Well, send me the code to review :)

@darkmode404 2 года назад

Not to disrespect but, I'm already appreciating that he uploads even one code review.

@raulrojas6552 2 года назад

extremely useful for Laravel newbies

@AbderrahmanFodili 2 года назад

This really helped reduce my imposter syndrome. I can sleep peacefully tonight unless I read a freelancing project requirements

@p30mehrdad 2 года назад

Yessss! Code review! thanks

@GergelyCsermely 2 года назад

Thank You, I like this videos, excellent help to learning

@sirajul-anik 2 года назад

For the recipes create/update operation I get his point for the duplication. He doesn't want to run queries twice. Your approach is okay. But I'd prefer to "make an array of common attributes, push conditional attributes to that array" and then create the row. With this approach, I'd need to run create the row only once and no duplication.

@ryanb509 2 года назад

I was thinking the same thing but I can do one better. There's no need to merge arrays even. If you add the create after the if statement for dealing with the image. The code uses the variable $name (though I'd call it something clearer like $image_name) but you can just include that in the array, if there was no image uploaded that variable won't get set and it will just be null.

@sirajul-anik 2 года назад

@@ryanb509 I am not sure if that's a good approach or not, because you're trying to access a variable that's not even initialized unless gets into the if block. and I don't think it actually differs from my approach. $attributes = ['a' => 'a', 'b' => 'b']; // add the common fields if (logic) { $attributes['images'] = 'value'; } Model::create($attributes); IMO, the above snippet looks what it means and I think your approach doesn't add any value. That's my opinion.

@LaravelDaily 2 года назад

Yes, I agree, good catch.

@ryanb509 2 года назад

@@sirajul-anik Both work. IMO I find the code for my method a little cleaner and more clear. For example if you are trying to quickly look into what is being passed into create, its right there. However with your method you have to go back in the code to where you are setting the variable then also look at where you are merging in the image to get the full logic. if ($logic) { $image_name = 'value'; } Model::create([ 'a' => 'a', 'b' => 'b', 'image' => $image_name ?? null ]);

@sirajul-anik 2 года назад

@@ryanb509 it's more of the developers taste. both works fine.

@alifallahrn 2 года назад

Thanks, the tips were great!

@salmamourad4407 2 года назад

thanks for the review we need this type of video to learn how to code pleaase make more review videos on your channel

@muzika5001 Год назад

Good explanation!

@emmanuelsiziba5142 2 года назад

thank you from Zimbabwe

@LoganathanNatarajanlogudotcom 4 месяца назад

Love this video. thanks

@Ehsankhan0577 2 года назад

thank you sir it's really helpful please keep it up

@asadeisa5591 2 года назад

this kind of video is very very helpful

@joelmnisi4649 2 года назад

With admin prefix, he can also create a new admin.php file on routes folder and separate admin routes from other users.

@belce1982 2 года назад

Thanks as always!

@drushtant 2 года назад

Thanks a lot. This is really very helpful. I just remembers I shared my codes with someone but forget to share the login details! and I know how irrirating it can be.

@alexaverkiyev9099 2 года назад

Worked with Laravel Love - very powerful package.

@mohamedkamal7606 2 года назад

Thank you, very helpful

@lahcenfahmi5361 2 года назад

Thanks so much for the video

@profesone 2 года назад

Helpful AF

@amarhidic2528 2 года назад

Image can be stored like this: 'image' => request()->hasFile('image') ? request('image')->store('images/recpies') : null So the final code can be even cleaner and we don't need to use if-else and update record after it has been stored. $recipe = auth()->user()->recipes()->create([ 'title' => request('title'), 'category_id' => request('category'), 'procedure' => request('procedure'), 'duration' => request('duration'), 'ingredients' => request('ingredients'), 'image' => request()->hasFile('image') ? request('image')->store('images/recpies') : null ])

@ejaadhulab8924 2 года назад

Thank you for sharing this code. I really needed this. I need some help in my code, can you pls help me out. I have Leaverequests table, $table->foreignId('leavetype_id')->references('id')->on('leavetypes')->constrained()->onDelete('cascade'); $table->foreignId('user_id')->references('id')->on('users')->constrained()->onDelete('cascade'); //User requesting leave $table->foreignId('handover_staff_id')->references('id')->on('users')->constrained()->onDelete('cascade'); $table->foreignId('approved_staff_id')->nullable()->references('id')->on('users')->constrained()->onDelete('cascade'); ListLeaves Controller, $leaverequests = Leaverequest::with('user')->latest()->paginate(5); I am unable to get the username for "handover_staff_id" and "approved_staff_id". I am already getting the "User requesting leave". I am unable to create the relationship between User modal and Leaverequests modal. Thank you in advance.

@whoami-so2hy 2 года назад

thanks for sharing

@mhmdomer 2 года назад

Thanks for the review! The slug attribute must be unique for each recipe because it is used as a key to get the recipe in the route model binding. However, in this implementation, if two titles are the same then there will be conflicting slugs because the slug is not unique at the database level.

@LaravelDaily 2 года назад

Good catch.

@marekjurica9098 2 года назад

I found this solution: protected static function boot() { parent::boot(); // registering a callback to be executed upon the creation of an recipe static::creating(function($recipe) { // produce a slug based on the recipe title $slug = Str::of($recipe->title)->slug('-'); // check to see if any other slugs exist that are the same & count them $count = static::whereRaw("slug RLIKE '^{$slug}(-[0-9]+)?$'")->count(); // if other slugs exist that are the same, append the count to the slug $recipe->slug = $count ? "{$slug}-{$count}" : $slug; }); }

@timsurbanovics9744 2 года назад

I would like to use spatie sluggable package to prevent such issues

@ngocatnguyen5771 2 года назад

You can get the data out of request first then using it to check if the is image in the request or not, then after that insert into the database. This will using only 1 query instead of 2 in case there is image. I was taught that limiting the db transaction as much as possible.

@ayomikunsamuel7743 2 года назад

I think in following the single responsibility principle, the code block to upload an image should be abstracted and just called when needed.

@LaravelPackageTutorial 2 года назад

Nice 👍

@mansoorkochy 2 года назад

if you follow junior code review as a series It would be appreciated, as well please make records on php design patterns Best regards

@brianotido4069 2 года назад

Thanks

@genechristiansomoza4931 2 года назад

The "duplicate code" refactor calls the database twice. What I will do instead is to construct first the array of parameters. If there's image then I will add it to array of parameters. Lastly, I will call the create method passing the array of parameters.

@PsyHooZ 2 года назад

I think in last example better to do this: $recipe = [all data in example]; Then if (image) $recipe['image'] = value; And then user()->recipe()->create($recipe) This optimize query to db, I think 🤔 You don't need create, then update, if you can once create)

@daniyarbaizhaksynov9230 2 года назад

Hi! Thanks for video! I think last case was wrong way of solve because in corrected version gets 2 request to database first for insert and second to update which is not cool! There have another ellegant way to solve it)

@WebJourneybd 2 года назад

Thanks a lot. Will you make a video.... How to delete database notifications by row in laravel?

@jsjunior 2 года назад

How did you put sidebar as an icon in Phpstorm? @Laravel Daily

@daleryanaldover6545 2 года назад

I remember my code looking exactly like that when I started using web frameworks, especially Rails and that habit of mistakes was brought in to my first Laravel project. lmao

@araromirichie4825 Год назад

I just wonder when will I be this good with Laravel 😢

@luizmeier 2 года назад

Exactly what happens when starting in a new company. Repositories without any instructions on how to install and configure things.

@timsurbanovics9744 2 года назад

For controller method ill prefer to make variable $data = $request->validated() in start of controller method. If(image) { logic $data['image'] = $name }

@gssj-o8p Год назад

Should I import the namespace of the class I use in route files?

@alexanderanastasiadis9284 2 года назад

It is necessary to declare the reference between fields in migration or we can put a simple integer and it can be done through a relation in the model. For example if i had relation between recipe and user from function that is a wrong practice?

@LaravelDaily 2 года назад

Watch this video: ru-vid.com/video/%D0%B2%D0%B8%D0%B4%D0%B5%D0%BE--77LnBSL2Zk.html

@rahman_abdu 2 года назад

Things to do before laravel application deploy

@randomguy5922 2 года назад

Thin Controllers Fat Models, do this principal applies for LARAVEL controllers also, because most of the time, I see thin Models and FAT controllers in laravel.

@aribiali3574 2 года назад

Hi , please tell me how can i get ur level , I'm lost What should i do ?

@vielta. 2 года назад

"I got used", proceeds to make a mistake.

@renwar 2 года назад

Can the last tip be written in a better way by using save() method instead of create()? Smth like this below $recipe = auth()->user()->recipes(); $recipe->title = $request->title; ..... Other fields..... If ($request->hasFile('image')) { $recipe->image =..... } $recipe->save();

@LaravelDaily 2 года назад

Probably, but not sure it would be worth this change, in fact it makes it less readable, in my opinion.

@sofyanmaulana8252 2 года назад

Yeah, I did the same too, because it only requires one connection to the database

@LaravelDaily 2 года назад

Yes, I agree from DB query perspective, good catch.

@kodaloid 2 года назад

The change you made to deal with the duplication doesn't look right to me. The original condition checks only duplicates because the coder didn't realise they could append to the array that gets passed to create(). I get that you fixed it by adding an update if an image was specified, but you created a new problem by adding a sql command that doesn't need to exist. I'd instead encourage prepping the data properly before sending to the db to be more efficient.

@abderhmandaya1655 2 года назад

Sorry, but when I repeat the code I see that it is better than doing 2 sql

@alexbaskov 2 года назад

disagree with the "duplicated code thing". you're trading couple of lines of code into 2 separate db queries which may become quite costly in some huge distributed db servers and cause unneeded delays. could just create an array first, and then based on conditions add an 'image' key for that image and still do a single 'create' thing. i see those issues all the time when some trivial data is being injected based on couple of different relations, and it's one insert followed by 4 updates of newly created record. imagine you will need to add some versioning/revisioning or custom logging on top of that, when "date_updated" changes 5 times in 2 secs and 5 records in the "actions log" db. hope it helps some new coders - always think about scaling and optimization. not about 8 lines of code instead of 10.

@Mr_JuanCarlos 2 года назад

Crack!

@haroldchoung3858 Год назад

omg, how to get the password? guess?

@ahmedhelal174 2 года назад

ru-vid.com/video/%D0%B2%D0%B8%D0%B4%D0%B5%D0%BE-twPM-6X0pMI.html Thank you but I have one comment for that You saved the recipe with query then if it has image you did another query You hit database two times (2 queries) I think there is another good approach for that how about?

@LaravelDaily 2 года назад

Yes, good catch,.other people also pointed it out in the comments, read other comments on this video.

@lassestube 2 года назад

It feels sloppy to just send code that isn’t working, I agree a bit disrespectful if you will.

@andreich1980 2 года назад

In the RecipeController@index It's not great implementation for view_deleted... They don't eager load category and user for trashed recipes... It should be like this $recipesQuery = Recipe::with(...)->where(...)->anything(...) if (/* view deleted condition */) { $recipeQuery->withTrashed() } return $recipesQuery->get() Or something similar.

@andreich1980 2 года назад

RecipeController@store They could get current user form $request->user(), no need to use auth() helper Save an extra query $recipe = ...make() fill(['image' => ...]) $recipe->save(); It's a bad way to store images. They should have a recipe_id prefix or be stored in the recipe_id folder (123_name.jpg or 123/name.jpg) There's a tiny chance that 2 different users create a recipe at the same moment and there'll be a collision. Laravel has a default behavior - it generates a random file name - this is better.