Laravel Junior Code Review: 12 Tips on Everything

[@marekjurica9098]

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]

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

[@guilhermemoraes4055]

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

[@abusayeedshakil9774]

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]

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

[@brandon_ab123]

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

[@MuhammadHaseeb4]

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.

[@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]

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]

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

[@armentorosyan8268]

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

[@kareemlateefyomi8690]

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

[@fandinurrokhim2615]

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

[@ayushmukherjee23]

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

[@benouattara6249]

what is you definition of junior ?

[@adityakadam2256]

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]

Yes, I agree, good catch.

[@VitaliyHAN]

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

[@ljubicasamardzic7819]

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

[@sirajul-anik]

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]

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]

@@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]

Yes, I agree, good catch.

[@ryanb509]

@@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]

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

[@raulrojas6552]

extremely useful for Laravel newbies

[@larrylague1359]

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

[@geneartista9714]

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

[@AbderrahmanFodili]

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

[@exikoz]

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

[@gtsmeg3474]

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

[@dhellly]

Thank you Povilas... Great Review!!!

[@emmanuelsiziba5142]

thank you from Zimbabwe

[@targetx1733]

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]

Well, send me the code to review :)

[@darkmode404]

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

[@joelmnisi4649]

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

[@salmamourad4407]

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

[@p30mehrdad]

Yessss! Code review! thanks

[@ayomikunsamuel7743]

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

[@alexaverkiyev9099]

Worked with Laravel Love - very powerful package.

[@asadeisa5591]

this kind of video is very very helpful

[@GergelyCsermely]

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

[@mhmdomer]

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]

Good catch.

[@marekjurica9098]

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; }); }

[@buddhasmoke3000]

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

[@jsjunior]

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

[@ngocatnguyen5771]

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.

[@amarhidic2528]

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]

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.

[@genechristiansomoza4931]

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.

[@drushtant]

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.

[@Ehsankhan0577]

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

[@muzika5001]

Good explanation!

[@LoganathanNatarajanlogudotcom]

Love this video. thanks

[@belce1982]

Thanks as always!

[@ryiad2010]

wonderful video

[@mohamedkamal7606]

Thank you, very helpful

[@alifallahrn]

Thanks, the tips were great!

[@PsyHooZ]

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)

[@lahcenfahmi5361]

Thanks so much for the video

[@profesone]

Helpful AF

[@daniyarbaizhaksynov9230]

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]

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

[@gssj-o8p]

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

[@aribiali3574]

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

[@whoami-so2hy]

thanks for sharing

[@randomguy5922]

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.

[@mansoorkochy]

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

[@LaravelPackageTutorial]

Nice 👍

[@brianotido4069]

Thanks

[@alexanderanastasiadis9284]

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]

Watch this video: kzbin.info/www/bejne/Y2iafaF4iLFlkM0

[@buddhasmoke3000]

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

[@luizmeier]

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

[@kodaloid]

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.

[@araromirichie4825]

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

[@renwar]

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]

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

[@sofyanmaulana8252]

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

[@LaravelDaily]

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

[@daleryanaldover6545]

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

[@abderhmandaya1655]

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

[@rahman_abdu]

Things to do before laravel application deploy

[@alexbaskov]

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.

[@avvay0]

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

[@eziosan7208]

Would you "Junior code review" my code, please.

[@LaravelDaily]

Maybe, if it's not a big piece of code. Send it to [e-mail] with information what exactly I should pay attention to.

[@haroldchoung3858]

omg, how to get the password? guess?

[@Mr_JuanCarlos]

Crack!

[@peppis3408]

its not disrespect at all, its still a junior code so you expect these kind of errors lol

[@ahmedhelal174]

kzbin.info/www/bejne/qqizfmBsjZWjg6s 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]

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

[@lassestube]

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

[@andreich1980]

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]

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.