Refactor "Senior" PHP Code with Early Returns

[@hansherrera5321]

Seeing this code coming from a senior dev makes me feel really good about myself

[@andibachtiar8788]

sometimes they just simply won't adapt to a new concept

[@ivanjelenic5627]

The person that wrote that code is not a senior dev. They might call themselves senior, but they're not.

[@PaulaBean]

Perhaps it was a purist who thinks functions are only allowed to have one return statement.

[@spicynoodle7419]

Senior dev as in 90yo PHP dev who's been coding for 2 weeks

[@lassestube]

😂same here

[@IlPandax]

I have 20+ years of experience. I met every type of developer you could ever meet. Once I worked with a senior developer who didn't believe in indentation. He wrote code on few lines and even mocked me when I indented his code every time I had to correct one of his bug. On another occasion, another colleague, I had to search for a bug in one of his controllers, that was more than 1000 lines long. In the past I made mistakes, of course, learning is a "long" process, but I always tried to do my best to write clean code. Sometimes other people just don't seem to care.

[@GerardoBelot]

Tell me about it, rigth now Im working on a proyect with clases with +3000 lines each, is a mess!!!

[@ariumboroseno]

@@GerardoBelot uwoww, is that a legacy project ?

[@souravdutt7]

Lol, I can feel it... In my freelance journey, I had worked and still working with a team of devs, where most guys really don't care about the pain of others. It mostly happens when the most senior one ignores it.

[@rikiamaru]

@@GerardoBelot i feel you bruh, im in the middle of refactor it also, and its a niggghhtttmaarreee

[@javieru5871]

Feel you bro, some people are just fine as long as the functionality works...

[@AranyaSen]

json_decode($response->body()) can be refactored into $response->json() Nice demonstration of how nested if-elses can be improved drastically! I hope the author would see this video and learn from it.

[@JoseDlucca]

I new versions of laravel? I think that Guzzle remove the json() method because it was not in the specs of PSR, but i am not sure if the laravel facade Http also remove the method

[@celucasa6810]

@@JoseDlucca I confirm $response->json() works just fine in Laravel version 10.12

[@spicynoodle7419]

You can even take some nested object in the JSON like this: $json = $response->json('rows.0.elements.0') and it will return null if that object doesn't exist

[@1337kaas]

​@@spicynoodle7419thanks for sharing 🙏

[@tomsriver2838]

Works with Laravel 8 too​@@celucasa6810

[@f13775]

Seniors sometimes write code which smells. But before making any conclusions I think there should be some clarification questions regarding this code, for example: - Does the code work? - How well does it work? - Maybe there was some deadlines to write this code? - Is release was more important than quality? You have to consider this and probably much more questions together. So "Keep Calm and continue refactoring your codebase to better code and design"

[@cichy86]

That is true, but if You are 5 levels deep in nested if/else then You should consider that something is wrong :) but as You mentioned there are multiple reasons for this code to exist

[@Asiro-S]

tbf, if your task wasn't to write shit, this code is not acceptable, because it is much easier to write better code than this.

[@anti5895]

Nah, I don't see any of these excuses valid for this case. This code IS garbage.

[@JackSoutherner]

One of the point of "seniority" is to know how to write clean code "already". I can't imagine that monster CEO who was standing behind, yelling "FASTER YOU BASTARD!!!!" and that "senior" suddenly forgot early returns concept. More logical for me is sabotage before quitting 😂

[@Atiradeonvideo]

People that blame a framework for that code simply lack in judgement and honesty. Every framework AND language allows bad coding.

[@BernhardK91]

Blaming the framework for bad code is like blaming a hammer not working as a saw.

[@ionelCristianLupu_]

Man, I just love these types of questions that need refactor. I would refactor code all day.

[@evol9013]

That means you are not stuck 😅

[@josibee]

I believe the only aspect I would approach differently is implementing a service class specifically for the Google Service. However, I must acknowledge that your version of the code is more streamlined and organized.

[@marcusgaius]

It should definitely reside within a service class, and extracting the code into the service is not in collision with having the streamlined and organized code.

[@ivanjelenic5627]

He probably didnt do it to keep the video short.

[@BrucePeterkin]

The early return / fail fast method resembles unit tests - getting your ducks in a row! I understand where these nested statements come from - it's "stream of consciousness" programming, but each "test" leaves a micro tech-debt to be resolved. Full marks to the original developer in this example, to test for each point of failure along the way. In writing either code blocks or unit tests, I like to begin with pseudocode - a comment on each line, reading like a checklist of things to verify, then coding beneath each one.

[@crispy-channel]

such a nice way to explain each and every aspects of line.

[@laubannenberg5446]

I would add one more change: use a different message in the last exception. The second exception (after the refactor) is when you don't get well-formed data in the response. That's different from not getting any UK destinations (the third exception). If all you have is an error in a log, it's nicer if you get a message like "no UK destination received", then you know where it went wrong.

[@shahsawoodshinwari]

I am not a senior developer, but I have had the chance of working on projects developed by seniors, which gave me such a great opportunity of refactoring, especially making controllers

[@devKazuto]

You could even simplify the joined if statement to just one check with null-safe operator: if ( empty($json?->rows[0]?->elements) ) { }

[@spicynoodle7419]

I would have done the same, except extracted only the elements since other parts of the response aren't used: $json = $response->json('rows.0.elements.0'); if (empty($json)) { throw... }

[@evol9013]

It only works on PHP 8 😢

[@jessedeboer2301]

I think the foreach could be changed with an in_array, that also can be early returned. Then at the end only return what you need so the logic is fully complete.

[@ste8701]

i don't think it would work as in_array will search for the whole destination value..but in this case you just want a part of the destination so strpos/str_contains is just fine

[@matthiashoffmann6555]

it is so satisfying how nested if-else code collapses with early returns...

[@adrianoalves-qripto]

I would use Laravel throw_if() function instead of the IF statements to make the code even more elegant, readable and somewhat with less lines letting it all in Laravel house of tools. Thank you for another Great Video!

[@omkarchoudhary1899]

Great learning be continue with these types of code refracting videos.

[@matrixlander85]

You could have moved the foreach into the IF block above with the 3 conditions and made them positive checks instead. So if all the checks passed, the loop would run and possibly return. Else, the Exception would be thrown at the very end. Just a minor style change and removes the duplicated Exception

[@alnahian2003]

My instinct was saying after seeing your retweet that you’re definitely gonna make a video on that 😂 Now here we are!

[@LaravelDaily]

Couldn't miss that one, huh! :)

[@alnahian2003]

@@LaravelDaily enjoyed every second of this video 💫

[@dawoodfaiz2338]

Thank God. I was scared of my coding style until I watched this video.

[@asfiaaiman3699]

Beautifully done by you!

[@al-assili-sounds]

You can search for the "UK" value in the destinations array by using the in_array function :), in stead of using the foreach loop :)

[@wgblondel]

This is not from a senior developer, this is from an impostor

[@He4vyD]

Senior is probably referring to the amount of years he has been programming instead his level of expertise.

[@wgblondel]

@@He4vyD Yes, definitely

[@bubblelong4759]

I googled the word "impostor", means liar。You think Laravel Daily is a liar? Why?

[@phoneywheeze]

​ ​@bubblelong4759 the commenter is talking about the person who wrote the code being an imposter(fake) as a senior, accusing them of not being as competent as a senior developer would be. the commenter has not made a comment on laravel daily

[@ste8701]

😂😅

[@LifeAquaticSteveZissou]

Excellent video. I always tell my web team "write your code under the assumption that the dev who inherits it is a homicidal maniac who knows where you live". 👏

[@giacomogaravaglia6742]

this video should be wateched by everybody join a company, on each level of seniority!

[@shubhamsahuSD]

वो क्या है ना, जो डिमांड होता है हैं, चर्चा भी उसी का होता है.. This is the demand and users of laravel framework.. 💙

[@joeythai1000]

Very good, I'm a php programer and I use laravel too, learning so much with your channel, my congrats. Sorry my english.

[@DimaSimonishvili]

issets can also be combined like this: ! isset($json->rows, $json->rows[0]->elements)

[@EndreVarga_rnd]

Thanks! Never found (or looked up) this in the docs, but it is! BTW you can only check the 2nd variable, as isset returns false, if the rows property or an index on it is not set, it does not throws exception. (It is also new for me, that isset is like nullsafe at any part, but tested in PHP sandbox now back to 5.2.0, it worked - never tried this before (~13 years in PHP development...))

[@DimaSimonishvili]

@@EndreVarga_rnd Haha there's always a room for an improvement.

[@mityukov]

I believe, there's ->json() method of the $response. Also, one could use Http:: get(...)->throw(). It wouldn't throw plain \Exception, however, but that could be even better (depends on the specification)

[@wadecodez]

one of my pet peeves is initializing values in the return statement. every time there is a bug involving the return statement, I'll have to create a new variable to print it to the screen or inspect it with the debugger. Then I'm left with a decision to delete the variable before making a commit. IMO you might as well create that variable the first time around so when bugs come up you don't have to think about cleaning up the lines modified during debugging. Also now that PHP 8 is mainstream, optional chaining and null coalescing should be used as much as possible. It is faster and safer to always expect null values rather than occasionally checking.

[@enriquerobledo845]

This kind of refactoring videos helps a lot thank you ;)

[@ScottHorsley]

I love using collections instead of an iteration to check values. Specially so I can perform a where () or even better, a first() to return the result.

[@GutoCmtt]

Wow, I'm amazed by the level of the comments here. Makes me feel good that I worked in a few places where developers were kind and cared a lot for helping and improving and trying to understand the situations we're at sometimes. The post itself mentions they received an application. That is kinda open, isn't that? If that's an application for a dev role there can be a looot of stuff underneath it. What was the deadline? Was it live coding? There are a lot of developers who crumble under the pressure of having someone on their shoulders, which is the same as live coding. Same goes for a very short deadline. How much code was expected in the given timeframe? There are a lot of job applications that give you a deadline that you are not expected to meet without leaving stuff behind, I've had a few in the past. Also the person exposing it was posting it to feel superior and get engagement from people that would also want that - which is pretty obvious given, you know, it's from twitter. Judging other people's skill for some random code you see without context isn't useful for anyone is not healthy at all.

[@SaiyanJin85]

Nice, it removes a lot of mental burden. One more thing I would is the foreach section I would extract it in a separate method let's call it UKDestination() and the I would move the last exception throw above, $UKDestination = $this->UKDestination($parameters); if ( empty($UKDestination)) { throw \Exception(''postal code not valid') } return $UKDestination Maybe it's just me but I prefer the happy path to be always at the bottom otherwise it messes my brain up . Thanks!

[@LaravelDaily]

Yeah, good suggestion! I was actually thinking to use some collection operation instead of a longer foreach loop. But separate method also makes sense!

[@SaiyanJin85]

@@LaravelDaily Thanks!

[@JouvaMoufette]

Also I have never seen anyone compare a variable to empty array. I'm not even sure if that reliably works! In some languages, the use of an empty array or empty object gives you back a newly allocated block of memory, and thus these checks actually fail. Just use empty() which also encapsulates the isset check

[@Daaboo]

He is most likely a senior javascript developer.... seen those empty arrays all over js code....

[@DougLopes]

Listening you was like: "i'm pretty sure, he is reading my mind..." haha Is great to know that i would refactor tha pretty same things of a experient programmer like you.

[@ercog7921]

I like to put the api url in the env file as well. Then make a separate class to communicate with the api, and make giveConfig binding through the container

[@SodalisUK]

IMO API URL should NOT be in .ENV but it should be in a constant string in the class..

[@ercog7921]

@@SodalisUK api url usually has live and sandbox. Therefore, in order not to make 2 constants and write if logic, it is easier to use env

[@SodalisUK]

@@ercog7921 I think the API URL should be local to the code that uses it. If there are two APIs, then it is fine to use the environment name from .ENV to decide which one to use.

[@RhythmnOfThought]

Theoretically, you could do an "early return" in the forach loop by reversing the if condition and using "continue" (instead of an actual return). If someone reading this thinks that would be considered a bad practice, please let me know!

[@ronssijei]

I have experienced this too. The senior who handled the app made the app like and feel like a wordpress :)

[@NoOorZ24]

Only thing that I'd change here is moving common part of that URL to .env file. By "common part" I mean everything from start to "api".

[@JouvaMoufette]

Yes! Then if there's a sandbox version of their api, you can point to that version easily

[@Teddykiss1996]

Add validation function for response))

[@rage13]

I think, since the if conditions only exists to throw exceptions, why not get rid of the if and use throw_if(), Code would be much shorter and meaningful Line 19-20 could be done like this, same with line 24-26 `throw_if(!$response->successful(), new \Exception('Error while getting data from google API'));` Great work though! I enjoy watching your video's, They always have something new that I learn from.

[@Atz22]

Nice video and explanation @pavel 🙂 - one question I actually exactly follow your approach for setting API keys, in env and config file then reference the config value...but I have seen other devs directly referencing the env value in the code e.g. in a controller or method using env('API_KEY_NAME') -- I wonder is this a bad practice or what might be the pros/ cons of this ?

[@LaravelDaily]

I think it's related to cache, with this approach you can't cache config.

[@vanosgarage]

You're doing it right! The devs you've seen are doing it the wrong way. Generally if you use env variables they should also be declared in a config file. Povilas is right, by referencing env variables directly you won't be able to cache them and in some server/PHP configurations you won't even be able to retrieve them using the env() method.

[@free2idol1]

@@vanosgarage thanks... May I know which server/PHP configuration where env() cannot be used?

[@michalgreksak]

@@free2idol1 I have VPS with almost default nGinx and php-fpm 8+ setup and I can confirm, that using env() in controller simply doesn't work. For example: dd( env('LOG_LEVEL') ) return null. I always use env() only in config files.

[@mohamadcheaib]

We can use also throw_if for one liner condition.

[@GloverHouse]

amazing, thank you for sharing

[@adilizm704]

i consider myself as junior dev but i will never write that code in the way that senior write it (WTF is that tree of if else ) from the biginning (3 Years now ) i learn from your code styles and im a fun of it simple, clean and easy thanks i may call myself senior after that haha love you 😅😅😅😅😅😅😅😅😅😅😅😅

[@bw7868]

I don't believe he's a senior developer except he believe he's senior because he's coding for 5 years or so. Also for Laravel yes sometimes I blame on Laravel because it's so easy that makes some devs may begin to use this tool before actually learning OOP and the language itself and they start working and destroy everything. But if we look into something like Symfony, it's so hard for any dev to start using it if he doesn't know OOP very well and the language itself, so we don't see this mess from a Symfony developer. That's my own opinion, the magic of laravel is a two edged sword.

[@mubasharrashid1201]

I strongly agree with you mate!

[@Novica89]

Again, it's not framework's fault because developer is not experienced. Framework itself provides tools for developing and what you said only proves that Laravel is much better developed and easier to setup and use than Symphony. I don't want to setup framework for hours, I want to use it to create my business logic and Laravel allows me to do so much faster than Symphony. If I was a bad dev, I would create the code in exact same way using Symphony as well, however, I'd have to annoy myself for hour setting it up to do what I want. There are thousands of bad devs using Symphony or any other framework, the same way as there are using Laravel. Bad devs are everywhere

[@mubasharrashid1201]

@@Novica89 But i think beginners should start there career on CORE Php or any other framework than Laravel for better growth and OOP skills, If a DEV start their career on Laravel it takes lot of time for them to learn good coding approach as laravel gives every thing without any effort. I totally agreed with @B&W on this

[@mubasharrashid1201]

@@Novica89 Im not against Laravel, I love laravel, I also start my career on laravel, but i always thinks I am weak in OOP concepts, and i think it could be better If i started my career on CORE PHP

[@Novica89]

@@mubasharrashid1201 I'd say you're somewhat correct :) Having a firm(er) grasp on PHP OOP would be the best possible scenario before using any framework, but there is also a LOT that you can learn about OOP as you start using an advanced framework like Laravel :) There are so many advanced OOP concepts in the core of Laravel framework that, as you read it's whole documentation (which i would strongly advise) sticks into your brain and makes you rethink what you did and how you did it before you started using Laravel :) If you are somewhat intermediate in PHP and have an "ok" foundation in OOP, you could start with any framework from that point on and just learn more about OOP as you go with the frameworks help. There is so much to learn from the documentation itself, not to mention Laracasts and similar websites.

[@coding_david]

Very nice refactoring! Thanks for the hint with Laravel Pint and the article! The original code was frightening though.. and by no means this was caused by Laravel :-D This is purely inefficient coding (no SOLID principle, no DRY etc.)

[@emekatimothyiloba699]

Thanks for all you do povilas...From the timestamp 2:41 can't I just use the global env function to reference my api key?

[@LaravelDaily]

Because config caching wouldn't work this way if you need it

[@NemanjaSokolovic-k6b]

And what in case if $json doesn't contain destination_address property somehow? That foreach loop is going to break. You either need additional check if it is set, or something like $destinationAddresses = $json->destiantion_addresses ?? []; foreach ($destinationAddresses as $destination) { ... }

[@7error8lade]

Nested IFs drive me crazy. It pretty much shows that the coder's approach to solving problem is to handle it sequentially instead of finding a pattern and grouping them together. If new criteria were to be introduced in the future, it will either add another level of IF or end up breaking the code, making the end result unpredictable. Also, in situations where database queries are involved, often the IF conditions can be coded directly into query builder instead of having to use an IF block to handle. e.g. $query->when($cond01, function ($q) { $q->where } . . . // or get / delete ->update([ . . . ]);

[@birsingh5388]

You fixed code like an artist ☺️

[@LaravelDaily]

Awww thanks :)

[@birsingh5388]

@@LaravelDaily Hi, I know Laravel little bit, can you please suggest me some roadmap or course for a beginner to develop projects and APIs in Laravel?

[@LaravelDaily]

Here's my public Laravel roadmap: laraveldaily.com/roadmap-learning-path

[@birsingh5388]

@@LaravelDaily Thank you so much, I will check that out 👍

[@SkySesshomaru]

You nailed it.

[@cichy86]

Great video, but do You really need multiple isset validation ? I mean `if (!isset($json->rows[0]->elements))` should be enough or am I missing something ? also whole json validation is strange in this code (for example there is no checking of destination_addresses existence but I suppose that You wanted to show how to refactor code not how to bug hunt

[@Rodrigoalvesdegois]

I might be wrong, but !isset($json->rows) || json->rows == [] might be replaced by empty($json->rows).

[@thirdvect0r]

if you use larastan then it will shout at you if you use empty() these days; so you should probably avoid it

[@oszi7058]

I'm a junior dev with no prior commercial experience yet just finished school. But I learned the concept of early returns a few months ago from this channel and I would have never written such unreadable and ugly code.

[@lynic-0091]

This channel has been recommended by 2 jobs I've worked at already. You're on the right path as a junior.

[@oszi7058]

@@lynic-0091 ❤️

[@DimaSimonishvili]

Don't judge like this. Today's young generation has way more available resources than it was 20 or even 10 years ago.

[@lynic-0091]

@@DimaSimonishvili As a programmer you're supposed to stay up to date. You can learn and master code style like this easily within a year or two.

[@DimaSimonishvili]

@@lynic-0091 I fully agree but not everyone has the same capabilities.

[@_whatistruth]

catching it and throwing it again! 🤣

[@AndrewJudd]

The capturing of the exception, and then rethrowing it as a more generic exception is actually hurting the code even more. You lose the granularity that exceptions provide, and more specifically the ease of handling elsewhere. Personally I would have made separate exceptions for the google api one and the postal code not valid that way you are able to capture it higher up in the code if necessary, and not rely on a generic catch(Exception) block and reading the message because what happens if the text changes slightly? Then your condition is now dead. Personally I would have bumped the double or (three conditioned if statement) into a private helper function because otherwise it feels like a run on sentence when reading it out loud. Instead, I would just have it named "hasValidResults" and go from there. Also, this really doesn't belong as a trait, at that point it may as well be a service that you import with an interface.

Second if can be replaced by empty($json->rows[0]->elements), also I'd import Exception. I'd also save destinations to a variable: $destinations = $json->destination_addresses ?? [];

[@prakctoleb1974]

why do you have conditions in the block || instead of && ? for example, if !isset($json->rows) returns false, what's the point of checking the same property further to see if it's an array?

[@lynic-0091]

If that guy is a senior then I'm the ruler of the galaxy. 🤣🤣 I'm afraid the client hired a junior who's pretending he's a senior.

[@sajjadhossainshuvo6975]

Thanks a lot.

[@dev22221]

if we see this type of code we give the previous developer benefit of doubt by saying, 1. perhaps he was not in a sound mental state when he coded, 2. his environment was not favourable. 3. maybe he had to finish the whole module by the eod, and never got the opportunity to return to it, lol

[@ste8701]

great refactoring! i have a question: is it really safe to save keys/passwords in a config file? isn't it better to have them in the db?

[@LaravelDaily]

That's your personal preference.

[@PacificDev]

Nice one!

[@MoemenGaballah]

Very Good Thanks.

[@salahradwan7091]

I would do the tow early if statements in tow lines without the brackets and save more 4 lines

[@PaulaBean]

I would also remove the curly braces, since they're not needed.

[@mohammadkhairi8014]

When we return in looping, it will automatically stop the whole the loop right?. not making the looping pending?

[@LaravelDaily]

Yes probably. But it depends on the situation.

[@jdrab]

I would ask how did this code get through any code review in the first place.

[@kingstalker]

in your opinion what is best to save address field in address column or separate them for country state street complex /house number etc....

[@LaravelDaily]

It depends on what operations you're planning to do with those address fields in the future. I usually keep them separately.

[@ericimperiale546]

How do you format blocks of code in PhpStorm?

[@LaravelDaily]

It formats it automatically

[@ericimperiale546]

@@LaravelDaily Thanks

[@rorumets]

Very nice ❤

[@kenjohnsiosan9707]

very helpful.

[@friedrichvillegasmurillo9775]

Code has to be at least readable, not that clean as many people say. Sometimes clean code is confusing, we have to find some reasonable balance between clean code and readable code. I know sometimes we code so fast and under pressure so it apparently work's, but we have to refactor the code, because probably later we have to change something and it becomes a nightmare.

[@antifa_communist]

Clean code and readable code are pretty much the same thing. The code at the start was neither, the code at the end was both. So I don't really get your point.

[@elgoogssie3969]

I would not remove the try/catch block. The rest is fine by me :)

[@beermarket9971]

the try catch and throw inside the catch killed me, i guess you could then try and catch that method and throw and continue until the end of times...

[@EmadHa]

I can't believe that this is a Senior code.

[@KG-id3hk]

The funny part is that Google Maps API resolves 99% of these issues in even fewer lines of codes.

[@JohnDoe-hs8np]

why iterating over $json->destination_addresses if you can just use one line of php internal function: _if ( _*_in_array_*_ ('UK', $json->destination_addresses) === true ) { ..._ 😉

[@souravdutt7]

instead of: if ( ! isset( $json->rows ) || $json->rows == [] ) can't we use? if ( empty( $json->rows) )

[@underflowexception]

empty checks if the value is null AND the if the array has elements, isset just checks if the value is set regardless of the value

[@arturpendrag0n270]

It should work fine with empty()

[@souravdutt7]

@@underflowexception Yeah exactly, it also works on true/false and 0/1, so if the concern is about making code shorter and readable, better to use single condition. I just mentioned as Sir @povilasKorop asked to share more suggestions..

[@Neptun084]

🤔 does being a senior synonym of perfect?

[@soniablanche5672]

why is the array being cast to object ?

[@99Spyder99]

Probably because he use the data in php, not outside, and he preffer calling it like $data->destination instead of $data['destination']

[@nithinjoseph182]

why not use \Throwable instead of \Exception ?

[@windallerox]

oh yes the old hadouken code

[@jmon24ify]

I hope the original author of that code watched this video and took some notes.

[@Andris_Briedis]

I don't like "return" in loops. I would put above the loop $destination = null; variable. And if not found (null) throw exception. After the end of the cycle or break cycle. And the found, required value would be left as the last return. p.s. url to env or at leas class constant.

[@MichalKuzmicki]

Do you mean the way of the "happy path", where your desired return value is always on the end of the method, so everything above is for operating on the data (like validating or transforming it)? For me this method is the best, coz it saves some energy for the future self: Imagine that now you need to do some more transforming within the loop - or even better: you want to move the loop insides into other method or even class - like a filter class, that it's main goal would be to get an object depending on the destination iso code. Something, that will allow you to get an object or in a case, there no condition will be met - exception or null value will be provided. Not only it will shorten the main code, the responsibility of the code will be spread more even, but also by using some KISS implementation you won't need to know exactly what happens in this new method, coz the name will cover it.

[@sho-ep6gy]

I honestly feel bad for whoever wrote this code, it's all over the internet 😅

[@syracuse4612]

need a sequence of senior code reviews

[@programmingmindset]

Senior developer with junior developer's flavour 🤪🤪🤪🤪

[@Jan-so5hm]

Is it not better to call that api on client side?

[@LaravelDaily]

That question is outside of the scope of this video, because I don't have more context about this project. It depends on the situation.

[@BlueJDev]

But making the request client side, eg using fetch API would expose the API key...

[@kennethkipchumba2532]

This mistake can be very costly.

[@mzerone-g6m]

I do not know how it call it senior i said alot says where are elite developer but just do not knowing any thing i was worked with company last year on laravel backend and it has like alot of problem in performance and in security and not just that but even on database level no relations at all

[@vNYCblade]

well its pretty clear that the person who wrote the original code was a Junior dev... or maybe even a STUDENT... or something... definitely NOT a Mid/Senior dev... the 3 levels of IFs for doing OR/AND checks is a clear indication of that.

[@darwincabarrubias1368]

this is how students code at school

[@ikarolaborda726]

The famous “Hadouken” of ifs… and the dev who wrote it is a senior???

[@bumblebity2902]

Why people blame framework for bad code? I have 2 takeways 1. Why enviroment values are received via config helper not env helper? 2. When can I make joint condition statements in early return approach?

[@LaravelDaily]

1. To cache the config 2. It depends on the scenario, there's no single answer

[@bumblebity2902]

@@LaravelDaily To avoid nulls in env values when is cached.

[@NikolaDjordjevic-ds6vn]

Wow this is my code... Joke of course 🤣

[@wowdesigns7436]

Xool

[@ionelCristianLupu_]

There is still a duplicated exception message

[@LaravelDaily]

Good point, another potential improvement.

[@JouvaMoufette]

Won't lie. If this "senior" developer gave me this as part of a code exercise for a hiring process, I wouldn't hire them. I cannot believe they wrapped everything in a try/catch only for the catch to rethrow it. Just leave it unhandled at that point! In fact this is worse! Now you don't know which line threw the original message!

[@AndrewJudd]

Nor the real type of the exception, all you get is an error message which is pretty useless.

[@JouvaMoufette]

​@@AndrewJudd Yeah if something else threw a non-generic exception, now you lost the type of exception. This code was just gross all around, and reminds me of a project I was a part of that started as a Proof of Concept/working prototype, and weeks away from the deadline we were told that it needed to be production ready. So everyone had to rush and made bad decisions, including suppressing all exceptions thrown to be a generic "an error occurred" message in the logs.

[@AndrewJudd]

@@JouvaMoufette So very true