Two Things Laravel Services Should NOT Do
Вставка
- Опубліковано 11 вер 2024
- Another video that improves on my previous video and takes one step further to create a better Service class, pointing to the non-ideal scenarios.
Related links:
- Original video: Laravel Controller: Move Logic to Action or Service Class • Laravel Controller: Mo...
- Laravel API 404 Error: Customize Exception Message • Laravel API 404 Error:...
- - - - -
Support the channel by checking out our products:
- Enroll in my Laravel courses: laraveldaily.t...
- Try our Laravel QuickAdminPanel: bit.ly/quickad...
- Purchase my Livewire Kit: livewirekit.com
- View Laravel Code Examples: laravelexample...
- Subscribe to my weekly newsletter: bit.ly/laravel-...
Yes, please, more like this! Learning how to do something is nice, but learning how to _think_ is very valuable!
Great video, because it illustrates some qualities any software engineer must learn to adopt in order to be successful: humility and humbleness. You weren't insulted by people's suggestions, but rather you were excited to show how they can be incorporated to improve what you've worked on. Great job.
Your these elaborations make me your fan.
Yes I totally agree with you, as I have working in Services, Repositories since long,
You could have just passed $request and access whatever you want, but that is prohibited, your services should accept parameters / data, not global context.
Example: STORE method of VoicesService could have accept Request $request, but you have passed correct multiple params that makes it ideal.
Because you have to consider writing TESTS, and you have to think TWICE before using Request, Auth, Session in services to make your test code smooth.
Respect form Pakistan.
I would suggest to also add the type of parameters for the methods, e.g.: store(int $question_id, ….
One rule I usually use is "when you first use a variable, make sure it is valid"
So in your VoicesService, on line 12 I would use firstOrFail(), and there especially because we have a request variable and a common rule is "don't trust user input".
And finally, I would set phpdoc for the method for typehint clearly because as that method could be use on another part except VoiceController, knowing the parameters and method description is ideal for not going to the service and remember which parameters are needed.
(sorry for long text, I usually speak french)
If you are using firstOrFail you can also register ModelNotFoundException in Handler as renderable and add some your custom messages or logic.
the validation is already done in the form request. There is a rule to check the ID (from what i remember)
@@giacomogaravaglia6742 in the whole video, I didn't saw the definition of form request. But if it checks the question di existence then ok
@@jacquesmatike9289 No it's not ok. Classes should not depend upon other classes to validate the data they receive, they should always do some validation themselves (the type of validation is up to the needs of the class obviously).
Having the controller check the existence is NOT enough, because the service does NOT know the controller did anything (or even exists).
Also, you lose reusability if the service can only be used in a context where its data was validated by this very controller.
So you're right, Jacques, to validate the data received in this service.
Just a side note: there is a throw_if helper like the abort_if helper that could remove the indentation from the if statement. In this case, it could be rewritten like the following:
throw_if($question->user_id == $user_id, new Exception('The user is not allowed to vote to your question'));
In case of throwing generic exception from service, abort functions can be used for shorter code as it will throw an exception anyway which can be caught in controller.
Nope. Because again if the service were called from somewhere other than a controller your service should not be able to abort the request. The service throws an exception and wherever you're calling the service from should handle the exception to do what needs to be done like aborting the request or throwing another exception.. etc.
This is a random video to comment this on but i really enjoy your content ive been following it for just over a year now and often come back for a refresher and seeing how you took your views comments and saw relevance and actually made a video showing even someone as experienced as you is willing to take advice from someone that might now have as much experience but a good point and make a video pointing out it was something you missed and or didnt think of and give someone else the credit then applying the suggestion and explaining why it may be a better approach just made appreciate your content even more THANKS!
Great idea to have these videos about best practices..keep it up ..another suggestion would be type hinting models or passing arrays as arguements to service functions so that the list of arguements is not too much .
I found this very useful, I’m constantly implementing bespoke solutions in my day job and I have written a load of procedural code in the past and placed in the controller. I have recently started to tidy this up but it can be quite difficult as some analysis is required to determine what needs abstracting in order to make the service globally accessible
Yes, it's one of the most difficult questions while architecturing the classes - "what needs abstracting".
Wow. Not only from watching the video, but I also learned something from reading the comments. Great video btw.
Nice Video. By removing auth() from Service you just explained S from SOLID. :)
Nice explanation, now the service is stateless and is just a use case which can be used anywhere.
If you ever run out of laravel specific content, it may be helpful to your viewers if you consider a "going back to basics of OOP" series. Things such as why there is a \ in front of Exception, or how to generally structure methods or go from one to another. Or even if you dont make a new course, having little reminders within the regular videos could be helpful to new programmers.
Or "think like a programmer" to remember to include things like exceptions (I'm going need to go back and add all that on my project) or anything else a new programmer may forget to include when coding.
Personally, I watch a lot of your videos and by the time I go to actually do some of these things on my own, I cant figure them out myself cause I don't remember things like the above mentioned.
And I like your teaching style, so If you do ever make such a series, I will definitely be watching. Till then, I guess I need to go find another good source of material to remind myself some of the basic php concepts other than endless reading of docs that to leave me more lost. you have any good video recommendations for the topic?
I think Laracasts series go back to basics: mobile.twitter.com/laracasts/status/1577768929459576833
Sorry I can't think about everyone what are they missing in knowledge but I'm always open to questions and comments
I wish I could work with you , so I could learn a lot from you . BTW I'm laravel developer and I learn a lot from you.
I would also use standard HTTP status Codes... like in this case if a user is not allowed to perform some action the correct status code is 403 (en.wikipedia.org/wiki/HTTP_403). Personally I don't like to explicitly return a 500. I would prefer to reserve those for unhandled exceptions.
I generally agree, but in this particular scenario, I was working with the API to refactor and I wasn't allowed to change the HTTP code return from 500 to 403.
So you can even improve this one further. Use a repository class to 'hide' all database related logic. And it would be nicer to use message bubbling. So even you service doesn't need to know what error a database will throw. And you can verify if the user can do something in Middleware.
In the context repositories are to handle database operation and so as the word, "database" used here, it's technical thing. Repository isn't used in this video. But i am saying about repository to clearify the concept of service.
Service classes have more relationships with business terms. So, people should name the class, even the method name, that's more related to business purposes.
If you think about any business, it can depends on another business. In such cases, depending on context composite service class can be created. That can be used for hand shaking between multiple businesses.
Amazing video. Of course this code can be improved, like every code, but the core lesson is very important and very well taught here in 8 minutes.
I feel like the first easy test of whether or not it's ok to do something outside of a controller starts with one simple question: If I wrote an artisan command that matched the flow of what the controller does, and called that function from in question from the command line script, would it still make sense? And would it still work?
So things directly utilizing the HTTP request/response system, or with a session (e.g. auth) should not go into model logic or service logic.
That's just one test but it often catches many of these cases
Yes, exactly this. A service that uses request or session context directly is hard to reuse in a different way. If I see things other than controllers using request or session, that’s a code smell to me. It seems sometimes there’s a lot of emphasis on having short controller methods. But I think in this case the “cost” of a slightly longer controller method is more than offset by the benefit of the reusable service gained.
Hello, great video ! Why not use the laravel Binding for the question and test if the user’s question is not equal to the user’s voice in the controller and then call the store with the question in parameter.
Instead of "if ()" you could just change "abort_if" to "throw_if". Make it's more readable in my opinion.
I would consider moving the authorization part in the service to policy or gate depending on the situation. If you are using a controller, that is where authorization goes for me.
I generally agree, but in this particular scenario, I was working with the API to refactor and I wasn't allowed to change the HTTP code return from 500 to 403.
I definitely agree on these principles.
Just a great video tutorial!
Thank you for your work!
I was thinking the question->user_id to auth->user()->id check can be extracted to a policy and checked in the request
Great video,Thank You!
I just love PHP community 💕 💕 💕 💕💕
Hello from Moscow, Russia, Povilas!
Thank you a lot for your videos, they mostly helped me to find a job one day :)
So as I mentioned above, I notice that at work we use Laravel Managers almost everywhere and every time.
Something like:
app(SomeManager::class, ['arg' => $value])->process($someVariables);
So I found out that it is pretty similar to Services, which I learned from you . What do you consider are Laravel Managers and which is the best to use?
Waiting for your answer, thank you in advance, you're great :D
Sincerely, Sergey.
I haven't seen this pattern like Laravel Managers, seems that it's some pattern used specifically in your company. Or maybe it's popular in Russia? But anyway, I don't have any opinion because I haven't used this pattern myself.
@@LaravelDaily i don't think it is only Russian way to write code. You try to dig deeper (google about it) and you'll find out that it used to be popular in Laravel but not now. I'm interested in what is your opinion about the difference between them? (Of course, only if you want to dive into it)
As I said, I don't have any experience with them, so I don't have any opinion.
Excellent Video
like ur videos, thanks for all
Yeah great video, more like this.
Please make a complete tutorial on Services in Laravel. Thank you
Search this channel, already a lot on this topic
What is best practice to pass much more than 3 parameters to service method?
you can make an array of them and just pass the array... I guess
You can make chainable functions in the service class, so you can use like this: $service->setYear(2021)->setUser(auth()->id())->store($data);
You can use setters for dynamic properties, here's my video ua-cam.com/video/BMHD5TPgEB0/v-deo.html
@@elektrodota I don't why, bud somebody told me this is not good solution
Thank you for this videos. So would you recommend dispatching a job in a service class or that should be a controller activity?
Personal preference.
It may depend on the case. if the job is related to the service task, like processing batches of data, that sound like something to handle inside the service, but if the job is about notify by email to users about the result, then I'll put those jobs in the controller
hello sir i use eager loading n+1 all good have 11 queries 68 models have less data in db maybe 20-30 n and open time 666ms and use 20 mb in debugbar what should I do ???
Nice 👍
nice...if using try catch in services...what am i suppose to return in catch...
There's no one rule here, depends on what you want to do next after the error.
@@LaravelDaily i see...coz my service use in both web and api controller..still finding proper way to cater this...thx alot..
I tend to return booleans from methods like this and throw exceptions in the controller. Is it better to throw the exceptions in the service class?
That's a personal preference, your way is also ok, in my opinion.
What if instead of passing in the question_id we would just pass in the Question itself. And get the question in the controller from the repository? That way the service wouldn't be concerened with the database query.
Yes that's probably a good idea. Although I'm not a big fan of passing full models to methods, as it's too much data, but in this case it makes sense.
👍👍
"User is not allowed to..." So this is authorization, you should use policies (or gate)
I generally agree, but in this particular scenario, I was working with the API to refactor and I wasn't allowed to change the HTTP code return from 500 to 403.
Why can't a service access another service?
It can.
First viewer 😍
7th comment. Getting closer to first
There is no argument type in function of service and also no error code in throw exception
Exceptions don't have error codes, they are not necessarily called for the API so no http status code required, or did I misunderstand?
With argument types, it's a personal preference whether to use that latest php syntax, personally I'm not a big fan, over 20 years of PHP I got used to everything being a vague type and variable name is what should describe the type for other developers
@@LaravelDaily putting argument type in function is best practice. But about http status code, shouldn't api response must be return with proper http status code like
403: authorized
404: not found
422: validation error
Etc....
@@LaravelDaily Argument type hinting has been there since php 7. It can hardly be considered "latest php syntax". It really helps if you work in a group. Your highly descriptive name may be a bit vague to others.
I would also annotate service method with @throws Exception. Otherwise someone who will use it may be in for a surprise.
@@SussanRai Service shouldn't care about http respose code. Service just throws an exception. It is conroller's job to handle the exception and decide what code to return.
@@alexrusin if you dont throw proper exception error code like new /Exception(403, "you are not authorized to update this model"), then how you can catch exception code like 404 , 401, 403 from service to controller. It will alwayz be 500 error code if the code is not sent in throw exception error
2nd
Should return 403, unauthorized.
He explained in prievous video, why he kept 500.
I generally agree, but in this particular scenario, I was working with the API to refactor and I wasn't allowed to change the HTTP code return from 500 to 403.
can you make video how to handle million of jobs in less time