Это видео недоступно.
Сожалеем об этом.

Laravel Model Method: Refactor into Service Class

Поделиться
HTML-код
  • Опубликовано: 7 апр 2023
  • A follow-up video on a recent one about a method for the price calculation in the model, based on your comments.
    Links mentioned in the video:
    - [Original video] Eloquent Accessor Needs Parameters: What To Do? • Eloquent Accessor Need...
    - [Premium Tutorial] Service Classes in Laravel: All You Need to Know laraveldaily.c...
    - - - - -
    Support the channel by checking out my products:
    - My Laravel courses: laraveldaily.c...
    - Laravel QuickAdminPanel: quickadminpane...
    - Livewire Kit Components: livewirekit.com
    - - - - -
    Other places to follow:
    - My weekly Laravel newsletter: us11.campaign-...
    - My personal Twitter: / povilaskorop

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

  • @al.e.k
    @al.e.k Год назад +4

    Hello!
    Martin Fowler wrote in his article "Anemic Domain Model" that "In general, the more behavior you find in the services, the more likely you are to be robbing yourself of the benefits of a domain model."
    Robert Martin in his "Clean Code" considers Active Record as some kind of DTO and that's why business logic should be placed in services.
    Programming is awesome )

  • @6daniel32
    @6daniel32 Год назад +6

    Amazing how you collected all the information in the comments and gave explanations for each class

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

    Thank you! Great discussions with the community.

  • @tetleyk
    @tetleyk Год назад +6

    I try to write my apps using the same structure each time. Interfaces, Service layer, DI where possible and so on, even if the app is a "throw-away" that is only going to be used once.
    Too often in my career as a developer one-off, throw-away apps have ended up being production apps for one reason or another and I do not want code out there that other developers can see and critique that has bad smell code.
    So, one-off or not, my apps are the best I can make them every time.
    In the case of the model in this clip, I'd put that method into a service class because it is business logic and not the responsibility of the model (remember SOLID).
    But, as you say, it's all about preference.
    As for comments about write now, refactor later, in my experience, this rarely, if ever happens especially once the app is in production.

  • @theceilidhboy
    @theceilidhboy Год назад +8

    This video is a great illustration of how service classes end up in the creation of anaemic domain models and procedural code dressed up as object-oriented code. The fact that the service class needs an instance of the apartment class in order to get at the prices is an example of a well-known code smell called feature envy. It’s a hint that something is wrong with the design. The price is intimately connected with the model and should be the model’s responsibility and thus encapsulated in the model class itself. By shifting it into a service class, you’ve just regressed to procedural code.

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

      With due respect, I don't think it's a good practice to put this logic in the Apartment model, I totally get your point but putting this kind of logic inside the model might lead to a code smell called large class, for now, it looks okay to keep it in the model but looking forward to the future and keep adding more methods with logic you will find the model be very very long. And remember models are responsible for many things already. unfortunately, I don't have any ideas for now than using an action or a service.

  • @JithranSikken
    @JithranSikken Год назад +3

    What I prefer: Contains the method logic for only the given model, as it does here, I include the method into the model class. (and if I have a lot of the same kind of methods and the model class gets long, i seperate them into traits as it comes closest to class partials).
    If the method uses more models for it's logic, returns a specific model controlled by it's input or returns more models, I include it into the service class.

  • @GutoCmtt
    @GutoCmtt Год назад +1

    It's all a matter of preference. That being said, dark theme on youtube is a blessing 😄

  • @Michielofzo
    @Michielofzo Год назад +8

    I think automated testing is also part of the reason why you should seperate the functionalities. I would love to see a TDD way of fixing this problem. I find it hard to come up with solutions for these kinds of 'small' problems myself as well. The term overengineering and spending too much time and 'hassle' on something as small as this feels wrong. But if you choose the 'easy' way out too often, your project becomes a mess slowly. Balance is key i suppose.

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

      I envy Devs that don't spend more than a minute to figure this out. In situations like this I see my self struggling but I have two options that gave same output

  • @DeTechDivus
    @DeTechDivus Год назад +7

    You can never please everyone, especially those with symfony or bigger background outisde of laravel. They will always complain about the structure. My initial thought is if you are on some good financial situation where you can spend a lot of time on researching and writing the best code ever that will work forever without refactoring good do it. But if you are low on time and finances, these things won't matter, especially in your own products that if they get a good financial support, you could always refactor it. I was always thinking about writing the best code etc and I never finished something because it takes a lot of time, but when I started doing it other way around I published few successful ones. Time is limited, knowledge is not and you can never do it all and get better results. Its just losing time. I like the method inside model, its much simpler

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

      I totally agree with you, sometimes you want to write the best code and to do that you spend a lot of time but at the end you just over engineering things.

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

      Pushing things out (too) fast leads to a problem that plagues most development projects. Unstable base upon which all other code lives, bugs, things not working as they should and complete stall of project later on in it's lifetime due to being unable to refactor, bugfix or implement new features due to former. Start off slow, making sure architecture is good, write tests for main features, refactor so it's all well made and then you can try and speed things up a bit

    • @DeTechDivus
      @DeTechDivus Год назад +1

      @Tutorials Guy no one said you should push something too fast and code rubish, i just said that trying to make a masterpiece code and taking things slow will almost never earn you anything besides some knowledge, think from the business point, while you are researching stuff and learning yourself how to slowly make some great code, someone else will steal your idea and do it quicker, sell it and then there is no point of your amazing code. Unless it is coding for nothing but self portfolio. World is full of developers and jobs are less available, why do you think you are better than 1000 others? Just because you take your time and write good code? Companies do not care about that and personally if someone told me he can do something in one week instead of one month i would go for it because in the end of the day, when most project get some high point you either sell them or refactor them. Thinking as hardcore developer wont make you money. I was in those shoes, tried to make something and while i do it slowly someone does it faster, even tho is looks like shit people like it because it is needed. Why would they jump to another one just because its coded better? They wont, same as for SAP its standard everyone use it even tho its developed 30y ago. Still rubish code but every single erp company use it. The best code doesn't get you money, it is sad but it is the truth

  • @kickass1179
    @kickass1179 Год назад +8

    When I wrote a raw query for a complicated logic, I was asked "why didn't you use eloquent relations?" and I also said "it is a personal preference, do as you like, as long as you get the results that you need and in a performant way".
    I never noticed this before, but in the last year or so, the Laravel community has become a bit "annoying" with "do it this way, not that way".

    • @Frank-ou2nr
      @Frank-ou2nr Год назад +2

      To be honest there is nothing wrong with the question itself in my opinion. I often ask the same when somebody does this, but not because I want to enforce the use of Laravel but just to learn from other coders. At some point you decided to go with a raw query instead of eloquent; knowing that decision is (for me) interesting knowledge and might result in some changes in specific scenarios.

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

      @@Frank-ou2nr sometimes, I would solve it quicker with raw query but 2 reason fitting Eloquent instead: readability for team mates not so sql advanced & second, the 'relational' output used later on frontend wich is more expected than a joined result, think here of tables id confusion

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

    And if the service class were a trait instead, then the model would use that trait. Of course, it would be a non-reusable trait, but it would concentrate the logic outside the model, and the access to these methods would be done as if they were inside the model.
    I don't know which naming would fit better than service.

  • @manishdeshpande
    @manishdeshpande 5 месяцев назад

    Nice perspective you've given. Thanks much 👍🏻

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

    i tend to keep these kind of `utilitary functions` inside models, but when it comes to business logic then i move to services
    dont think its black or white aswell

  • @CoffeeCala
    @CoffeeCala Год назад +3

    Nr.1 Answer from Lawyers and Dev-RUclipsrs: It depends...
    😄

  • @Bbtechjourney
    @Bbtechjourney Год назад +1

    In situations like this, when a logic are relating to a Model and does not depend on third parties, I use trait, achieve the same thing with your preferential method and as well keeps the model neat and shorter.

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

      but Trait is more of re-usability, in this case this calculation method is only for Department model.

  • @LeandroTabaj
    @LeandroTabaj Год назад +3

    Given a more real world scenario, this price logic might be way more complex. You may want to calculate price not only for dates, but also add promo codes, other types of discounts, some high demand dates more expensive... It make sense to have that logic in a separate class. For this example, one method, it doesn't make a difference

    • @lex224ification
      @lex224ification 6 месяцев назад

      Would it make sense to make methods like calculatePrice static within a service class?

  • @bn1andy
    @bn1andy Год назад +1

    i whit put the calculatePriceForDates method in to the resource class as a private method since that's the only place it's being used

  • @bulent2435
    @bulent2435 Год назад +4

    If you pass $this, then no need to put it anything in the model to call the class from resource. I would make it a static class and pass the price instead of whole model.

  • @user-bu7ck7cm9j
    @user-bu7ck7cm9j Год назад +2

    Thank you for the video. Why don't you add strict types declaration for your methods params? This would be some kind of a best practice usage .

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

      Old PHP habits die hard :) I guess I never encountered issues because of wrong/unspecified parameter types

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

    You can use trait, in this case will separate the code to different class file and will keep it as is , by the way I use model but when be more than 3 I move them to trait

  • @PunyFlash
    @PunyFlash Год назад +5

    Or just use trait in model and forget about service hell. As for me service is something that performs some logic that is not fit within models. For example getting data from external API, doing some file manipulation or generation etc. But definitely not manipulating single model data, nono. With active record you kind of expect that all model logic is fit within model itself. There are traits which much more clean than services in that case

    • @DeTechDivus
      @DeTechDivus Год назад +1

      Traits are mostly for reusability, no point in making trait for one method, it could have less code than with service class but still its like overthinking about it

    • @PunyFlash
      @PunyFlash Год назад +1

      ​@@DeTechDivus well the same point about creating service for 1 method. I talk overall. Traits are much cleaner way to store logic within models then services

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

      @PunyFlash i totally agree i am just daying for one-two methods it overthinking. Especially for newcomers, its harder to navigate through 10 different files for few additional methods that to have it some where they do not belong.

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

      You can extract method in ApartmentRepository class

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

      @@Alecs91 repository is anti-pattern in laravel.

  • @andrewvasiliev4548
    @andrewvasiliev4548 10 месяцев назад

    This topic is all about layers. Price method should be the part of business layer (hope noone gonna controvert). If you use models for db reflection only, ofcourse this method should be separated from model. How exactly this should be done - its all about personal preferences. All ways would have pros and cons.
    In fact, the best way should be in specifying abstraction "Can be price calculated by dates" (have no idea how to name it properly), making model inherited from AppartmentModel and that abstraction, and Calculation Service should use that abstraction as a parameter. One thing, that code would be mess, but realy extandable.

  • @bumblebity2902
    @bumblebity2902 Год назад +1

    It depends ... as long as you can make that testable. Period. Strangely, you didn't mention in this video.

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

    Only refactor when the codes start to gross out, at early stage of the development keep it simple first.
    when it's getting large, then start to refactor into service/action/observer/trait, totally depends on situation

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

    I'm a big fan of Services but in this case I totally agree with Povilas.

  • @nargizok5232
    @nargizok5232 Год назад +1

    What if you have other models than Apartments having the same method?

  • @iSensetivity
    @iSensetivity Год назад +1

    For my personal opinion it should be a ApartmentService with public method getList() and protected calculatePrice(). And method getList should return some entity class with all fields witch you need including price that was calculated in service.

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

      Ain't you passing the model as a parameter in the getList() method or in the constructor? it looks similar to Povilas's refactored code or even more hectic

  • @htetaunglaing6076
    @htetaunglaing6076 Год назад +1

    In my opinion, I'll use helper function ( I think you only need some calculations that are not complex so much ) or traits as second option.

  • @daniyarbaizhaksynov9230
    @daniyarbaizhaksynov9230 Год назад +1

    I think you should use service class only if method uses without model in the future.

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

    How would you handle the situation where you have multiple attributes on a model that rely on the current user. When you for example want to query 50 instances of this model, you'll query the user for each attribute on each record for each user, which will lead to a lot of queries. What would be a good way handling this?

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

      If I understood that situation correctly, then the solution is eager loading and loading all required attributes and their relationships at once, with one query

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

    Which is better to use DDD or use class services with model ? and how l know cases use it ?
    Thanks

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

      Personally I don't use DDD and don't have cases for it

  • @VadimBesedin
    @VadimBesedin Год назад +1

    Structuring and refactoring should be done to make code and work with code better, cleaner, simpler, easier, faster, etc. And not just for a sake of structuring and refactoring.

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

    what about trait in model i prefer to use it with use trait and write function in Trait

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

      Also possible, it's your personal preference

  • @shubhamsahuSD
    @shubhamsahuSD Год назад +1

    If you are in my situation what you will do, I always be a man on fire due to deadlines, in this case i prefer the fastest way.😂😂

  • @danglendedo
    @danglendedo Год назад +1

    I would keep it in model, don't like to separate logic from it's specific data. The method and model are still themselves understandable and maintainable. If they are separated, then me and teammates have to remember where the method is, don't forget to passing "this", more documentary, more couples (ApartmentService need ApartmentModel which must have price property, then may lead to ApartmentInterface 😅). This is just a small function, theories and best practices should be used, but not always. Ahhh it reminds me of Magento, feeling exhausted 😅

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

      Can you explain why ApartmentInterface is needed? What's design pattern is that?

    • @danglendedo
      @danglendedo Год назад +1

      @@free2idol1 because when working in team we should not assume that an object has some methods. In order to make sure a model's method is available whenever a service calls it, an interface should be implemented, a service shouldn't know what instance of the model, it just needs to know the model has the methods which the interface provides.

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

      @@danglendedo Thanks... So whenever a service is created, there must be an interface coming along? Or it depends?

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

      @@free2idol1 it depends. If your class is supposed to be used by other external codes or clients it will need an interface to assure methods and return types. If your class is used by internals or just you only, you can skip the interface. Personally, I prefer to have an interface so IDE can suggest method and return type in case i forgot what i wrote.

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

    Can we use a trait ?

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

      Yes but why?

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

      Because in my case i would prefer to use traits for a single method rather than having a dedicated class. and like this i can only use the trait inside api resources rather than init new object from a service class. Same things for the controller

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

      Traits are supposed to be used for REUSABLE code pieces for multiple classes. But yeah, if it works for you, you can use Traits.

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

    Whenever I see someone make a service, I often see a more Laravel'ey way to code it. In this case, a query scope:
    ** I also like explicit types, so used a CarbonPeriod...
    class Apartment
    {
    public function scopeWithPricesSumWhereInPeriod(Builder $builder, CarbonPeriod $period): void
    {
    // Dynamic name is optional, but allows you to load many periods at once
    $key = 'prices_sum_price_' . $carbonPeriod->first()->format('Y_m_d') . '_to_' . $carbonPeriod->last()->format('Y_m_d');
    $builder->withSum([
    "prices.price as $key" => function(Builder $builder) use ($carbonPeriod): void {
    $builder->where('start_date', '=', $carbonPeriod->last());
    }
    ]);
    }
    }

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

      It would be interesting to see if laravels API resources can conditionally add a dynamic sum/ average/ other aggregate method!

  • @Laflamablanca969
    @Laflamablanca969 Год назад +3

    Eww