Laravel Code Review: Optimize Queries and Other Performance

hello guys today we'll have a code review of this Artisan command for performance and other issues so this code was sent to me by email but before we go into the code review I want to reflect on laravel live UK so I just returned from there so you can probably hear my voice still recovering but it's okay now much better than immediately after the conference because I had so many talks so many great conversations as usual I go to conferences more for networking and for social stuff than the presentations so it was so cool to again feel the vibe of people I work for so I don't have employer I don't have a boss but you are my bosses and I work for you to teach you laravel and provide best practices in my opinion and one of the conversations was special to me personally I finally met in person with Dan Herring from filament also I met Zeb from the team of filament and now I have this official t-shirt of filament and you should expect more content for me on filament especially since now we have dates for release of Live Wire version 3 which is July 20th and filament version 3 which is August 1st so end of the summer you should expect more from me about that and actually while speaking to you at larva live UK quite a few of you mentioned that from my content you specifically enjoyed code reviews so why don't we start the first video after I'm back here with code review so the situation for this code review and the problem that the person asked me to solve so there's an artisan command to handle the attendances of the users which runs every minute and logs the users who don't have attendances records yet it's like for employees with shifts and checks if it's not weekend or holiday and then adds the data whether it's weekend holiday or regular date and if the person is not within 30 minutes of their shift then they are recorded as absent and then all that data is inserted with one query but that command is run once every minute which may be a performance issue so the question to me was can I optimize that somehow and run it differently to have better performance and have a few things to comment about this code and suggest something in addition to the main question so first let's put the main question aside and just go through that code for quick refactoring wins that I would change first quick thing I don't see the point in this temporary variable of weekend because it's not used anywhere else so I would do it inline like this it's a personal preference but if the variable is used only once immediately I don't see much point in that then phpstorm underlines that and let's read the message array push with single element so phpstorm detected that it's worth to use just data with brackets or whatever they called and let's click this and let's see how phpstorm simplifies that so three examples of the same thing instead of array push you can just do this thing if you have only one element to push in this case all three are now not underlined anymore then also I would probably do that else if also in line so if else if and then else if here like this without a new layer and reformat it to the left like this so this is more readable to me personally again quite a lot of things that I do is my personal preference you may argue with me in the comments and let's discuss but this is what I would do and finally in this block I see repeating data with ID which is generated every time even if there is no new record is passed to the data so there is a possibility quite a big one actually that it's not weakened it's not holiday and nothing to see here either but even in that case we generate order the uad so I think we need to generate it only if we actually need it and there are a few ways how to refactor that but I would just cut and paste it here each time ordered your uid like this and then we don't need that variable at all and we generate the uuids only when we need them and then if we go down to this finally in this Artisan command I would change this line after insert because see what happens you show the info that something is inserted before it is inserted so what if this fails for whatever reason so I would just change this here okay so these are obvious things that I just noticed within this Artisan command now let's get to the main question how to optimize that in terms of performance first I would say something maybe a bit controversial and we can argue you in the comments and discuss with you but I don't see anything necessarily bad in running one Artisan command every minute which runs two queries to the database so two queries per minute is not a performance issue in real projects there are many more performance issues with pages and endpoints running every second with multiple queries so this should be your focus in my opinion I wouldn't refactor the architecture of this solution just to save two queries in the database to me it's totally okay to run this especially if we optimize those two queries so the main two queries to the database if we for now skip the insert part which actually happens pretty rarely but to select the data we have two queries this one select the users which don't have attendances records and then select the holiday so let's focus on optimizing those two this one is a clear candidate for caching so the parameter for that query is just the date of today so the result of that query changes only once per day so we can totally replace that to cash remember or maybe even remember forever with the key of the date we have the key and then we have a callback function and inside of that callback function we cut and paste this return holiday query now the key would be holiday for that date so is holiday Dash this so it would run only once the first time for that day and then it would be always in the cache instead of the database of course it's only one way to name that cache or maybe you could even store that in the config for upcoming months or pre-generate that somewhere but my point is this should not touch the database every time every minute and then we get to this query I see two things to improve here first we get the users but what we really need is not the full user object we return users with shift with all of The Columns of both tables and if we scroll down what do we actually use from there for each user we use only user ID from users table and from shift we use only and add that's it so we need two columns from two tables and not just querying everything and another thing where doesn't have is not the best in terms of performance I have a separate video and I will link that in the description below why where has is typically slower than doing join in the database with query Builder so this I would change that query from eloquent to query Builder filtering only those fields that we actually need something like this so I've tried to write that query in the background to save you time and by the way I haven't even run that project so this may be with errors because I don't have the data in my database I just have the code from the author but it should be something like this again if you see any errors or mistakes put them in the comments but the principle is that you don't use eloquent with where doesn't have you use Query Builder select only the fields you need and you use joins to have it all in one SQL query otherwise where it doesn't have runs two SQL queries and scans all the table of attendances in in this case we join the attendances but we query only those records of users that have attendance as ID of no I hope I'm right here maybe it could be optimized as well but my goal is for you to get the whole idea that we don't use eloquent Warehouse we use join with query Builder instead so I guess these are all my thoughts about this piece of code if you have something to add to help the author maybe you would do that differently or maybe you encountered that specific issue of attendance every minute or shifts for employees in your projects shoot any advice in the comments below and as I often do at the end of the video I remind you that you can also subscribe to premium membership of laravel dailycom there is a new course released silently released just last week and I will talk about that in upcoming videos this week so you can access that if you're a premium member as well as premium tutorials which will keep releasing every week and a lot of things are planned ahead that's it for this time and see you guys in videos

Channel: Laravel Daily

Views: 10,637

Keywords:

Id: hgPu3l_HOBI

Length: 9min 33sec (573 seconds)

Published: Mon Jun 26 2023