Two Things Laravel Services Should NOT Do

Поделиться
HTML-код
  • Опубликовано: 10 авг 2021
  • 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.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
  • ХоббиХобби

Комментарии • 106

  • @travholt
    @travholt 2 года назад +43

    Yes, please, more like this! Learning how to do something is nice, but learning how to _think_ is very valuable!

  • @iedi3
    @iedi3 2 года назад +18

    I would suggest to also add the type of parameters for the methods, e.g.: store(int $question_id, ….

  • @nabeelyousafpasha
    @nabeelyousafpasha 2 года назад +7

    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.

  • @frankincredible
    @frankincredible 2 года назад +12

    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.

  • @tech-trials
    @tech-trials Год назад +1

    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!

  • @KastsEcho
    @KastsEcho Год назад +2

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

  • @jacquesmatike9289
    @jacquesmatike9289 2 года назад +7

    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)

    • @furgon6721
      @furgon6721 2 года назад +3

      If you are using firstOrFail you can also register ModelNotFoundException in Handler as renderable and add some your custom messages or logic.

    • @giacomogaravaglia6742
      @giacomogaravaglia6742 2 года назад

      the validation is already done in the form request. There is a rule to check the ID (from what i remember)

    • @jacquesmatike9289
      @jacquesmatike9289 2 года назад

      ​@@giacomogaravaglia6742 in the whole video, I didn't saw the definition of form request. But if it checks the question di existence then ok

    • @zHqqrdz
      @zHqqrdz 2 года назад +1

      ​@@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.

  • @ashay191
    @ashay191 2 года назад +2

    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 .

  • @hurleyd9828
    @hurleyd9828 2 года назад +5

    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.

    • @hossamabdelazeem9264
      @hossamabdelazeem9264 2 года назад

      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.

  • @zHqqrdz
    @zHqqrdz 2 года назад

    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.

  • @_passby5399
    @_passby5399 2 года назад +1

    Wow. Not only from watching the video, but I also learned something from reading the comments. Great video btw.

  • @JigarDhulla
    @JigarDhulla 2 года назад +2

    Nice Video. By removing auth() from Service you just explained S from SOLID. :)

  • @eloquent2200
    @eloquent2200 Год назад

    Thank you for your work!

  • @michaeldeeming4643
    @michaeldeeming4643 2 года назад +2

    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

    • @LaravelDaily
      @LaravelDaily  2 года назад +3

      Yes, it's one of the most difficult questions while architecturing the classes - "what needs abstracting".

  • @rakeshthapa9541
    @rakeshthapa9541 2 года назад +2

    Nice explanation, now the service is stateless and is just a use case which can be used anywhere.

  • @Somcoders
    @Somcoders 2 года назад

    Great video,Thank You!

  • @nirajgautam403
    @nirajgautam403 2 года назад +1

    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.

  • @behzodjon
    @behzodjon 2 года назад +1

    Just a great video tutorial!

  • @TheILOVEYOUXD
    @TheILOVEYOUXD Год назад

    like ur videos, thanks for all

  • @lassestube
    @lassestube Месяц назад

    I definitely agree on these principles.

  • @jonathangobiel523
    @jonathangobiel523 2 года назад

    Excellent Video

  • @methos02
    @methos02 Год назад

    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.

  • @wausiee1991
    @wausiee1991 2 года назад

    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.

  • @bensouthall6705
    @bensouthall6705 2 года назад

    Yeah great video, more like this.

  • @jcc5018
    @jcc5018 Год назад

    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?

    • @LaravelDaily
      @LaravelDaily  Год назад

      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

  • @mohammadashrafuddinferdous8649
    @mohammadashrafuddinferdous8649 2 года назад

    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.

  • @alila3883
    @alila3883 2 года назад

    Nice 👍

  • @UnknownUser-ud1es
    @UnknownUser-ud1es 2 года назад

    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.

    • @LaravelDaily
      @LaravelDaily  2 года назад +1

      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.

  • @maquiavelotube
    @maquiavelotube 2 года назад +1

    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.

    • @LaravelDaily
      @LaravelDaily  2 года назад +1

      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.

  • @JouvaMoufette
    @JouvaMoufette 2 года назад +2

    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

    • @JimOHalloran
      @JimOHalloran 2 года назад

      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.

  • @AbdulKudusIssah
    @AbdulKudusIssah 2 года назад

    Thank you for this videos. So would you recommend dispatching a job in a service class or that should be a controller activity?

    • @LaravelDaily
      @LaravelDaily  2 года назад +1

      Personal preference.

    • @maquiavelotube
      @maquiavelotube 2 года назад

      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

  • @yacobee
    @yacobee 2 года назад

    Please make a complete tutorial on Services in Laravel. Thank you

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

      Search this channel, already a lot on this topic

  • @sokotoful
    @sokotoful Год назад

    I was thinking the question->user_id to auth->user()->id check can be extracted to a policy and checked in the request

  • @iamriwash7943
    @iamriwash7943 2 года назад

    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 ???

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

    👍👍

  • @MrMatni45
    @MrMatni45 2 года назад

    nice...if using try catch in services...what am i suppose to return in catch...

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

      There's no one rule here, depends on what you want to do next after the error.

    • @MrMatni45
      @MrMatni45 2 года назад

      @@LaravelDaily i see...coz my service use in both web and api controller..still finding proper way to cater this...thx alot..

  • @PostScripton
    @PostScripton 2 года назад

    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.

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

      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.

    • @PostScripton
      @PostScripton 2 года назад

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

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

      As I said, I don't have any experience with them, so I don't have any opinion.

  • @seifobeid6545
    @seifobeid6545 2 года назад +1

    I just love PHP community 💕 💕 💕 💕💕

  • @rokzabukovec4685
    @rokzabukovec4685 2 года назад

    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.

    • @LaravelDaily
      @LaravelDaily  2 года назад +1

      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.

  • @daihop
    @daihop 2 года назад

    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?

    • @LaravelDaily
      @LaravelDaily  2 года назад +1

      That's a personal preference, your way is also ok, in my opinion.

  • @vitaliyp.3007
    @vitaliyp.3007 2 года назад +1

    What is best practice to pass much more than 3 parameters to service method?

    • @HamzaBhatti54
      @HamzaBhatti54 2 года назад

      you can make an array of them and just pass the array... I guess

    • @fadlulahmad7205
      @fadlulahmad7205 2 года назад +1

      You can make chainable functions in the service class, so you can use like this: $service->setYear(2021)->setUser(auth()->id())->store($data);

    • @LaravelDaily
      @LaravelDaily  2 года назад +1

      You can use setters for dynamic properties, here's my video ruclips.net/video/BMHD5TPgEB0/видео.html

    • @vitaliyp.3007
      @vitaliyp.3007 2 года назад

      @@elektrodota I don't why, bud somebody told me this is not good solution

  • @mannusaraswat3864
    @mannusaraswat3864 2 года назад +1

    First viewer 😍

  • @ClCfe
    @ClCfe 2 года назад

    "User is not allowed to..." So this is authorization, you should use policies (or gate)

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

      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.

  • @mohamedshuaau632
    @mohamedshuaau632 2 года назад

    7th comment. Getting closer to first

  • @devKazuto
    @devKazuto 2 года назад +5

    Instead of "if ()" you could just change "abort_if" to "throw_if". Make it's more readable in my opinion.

  • @ViperNerd
    @ViperNerd 2 года назад

    2nd

  • @RavnosPKR
    @RavnosPKR 2 года назад

    Why can't a service access another service?

  • @SussanRai
    @SussanRai 2 года назад

    There is no argument type in function of service and also no error code in throw exception

    • @LaravelDaily
      @LaravelDaily  2 года назад +1

      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

    • @SussanRai
      @SussanRai 2 года назад

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

    • @alexrusin
      @alexrusin 2 года назад +1

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

    • @alexrusin
      @alexrusin 2 года назад +2

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

    • @SussanRai
      @SussanRai 2 года назад

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

  • @TariqSajid
    @TariqSajid 2 года назад

    can you make video how to handle million of jobs in less time

  • @marklesterbolotaolo8259
    @marklesterbolotaolo8259 2 года назад

    Should return 403, unauthorized.

    • @bumblebity2902
      @bumblebity2902 2 года назад

      He explained in prievous video, why he kept 500.

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

      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.