Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor business logic to a service layer #433

Closed
joemasilotti opened this issue May 20, 2022 · 10 comments
Closed

Refactor business logic to a service layer #433

joemasilotti opened this issue May 20, 2022 · 10 comments

Comments

@joemasilotti
Copy link
Owner

I attended a talk at RailsConf that really got me thinking. Your Service Layer Needn't be Fancy, It Just Needs to Exist from David Copeland, author of Sustainable Web Development with Ruby on Rails.

The gist was that all of your business logic is scattered across your app. If you need to know what your app does you have to open models, controllers, jobs, and maybe even some views. Instead, he proposed moving all business-specific code to a service layer.

Note "layer" and not object. He doesn't care how you do it (POROs were recommended) just that it has to exist. And it doesn't need to be complicated.


Based on that I'm inspired to try and consolidate some business logic of railsdevs into services/. I kind of already started this with #387 but it could be extended further. Here are some high-level guidelines for what I'm planning.

  • Move non-database-related logic out of Active Record models
    • Exception: keep validations in the model
    • "Creating a user via the website is not the same adding a row to the database"
  • Move business-related logic into a PORO under services/
    • CreateDeveloper#create_developer(developer_params) is better than DeveloperService.create(params) - explicit about exactly what each class and method does
  • Service layer could be moved to a Ruby command line app
    • Return a state and the model, let the controller switch on that and generate the path
    • Return an error message, let the controller shove it in the flash
  • Roll our own authorization
    • Consolidate logic into specific services, but Pundit has proved too limiting
@aquilarafa
Copy link

Have you seen u-case?

@joemasilotti
Copy link
Owner Author

Have you seen u-case?

I haven't, no! I'm trying to stay as "boring" as possible right now but I will take a look for inspiration.

Do you use it in your projects?

@joemasilotti joemasilotti mentioned this issue May 21, 2022
14 tasks
@aquilarafa
Copy link

Have you seen u-case?

I haven't, no! I'm trying to stay as "boring" as possible right now but I will take a look for inspiration.

Do you use it in your projects?

Yes. I started "boring" as well. Once a controller starts to be more complex than simple operations I eventually extract to a domain layer with usecases.

@aquilarafa
Copy link

Uploaded using RayThis Extension-11

A usecase flow is made of "micro"cases or steps. It's very readable. I also like the functional style to unfold results (Success or Failure objects).
Uploaded using RayThis Extension-9

@jacobdaddario
Copy link
Contributor

That's pretty slick, and I could see it working really well in super-large applications. That said, I wonder if it becomes a bit of an over-abstractions when you're just dealing with 5-10 people contributing at any given time.

@aquilarafa
Copy link

This project

That's pretty slick, and I could see it working really well in super-large applications. That said, I wonder if it becomes a bit of an over-abstractions when you're just dealing with 5-10 people contributing at any given time.

I'm new to rails. Didn't build a lot of projects but I think it brings convention and good design to bigger teams.

@aquilarafa
Copy link

aquilarafa commented May 21, 2022

For the 'create developer' task I know that a CreateDeveloper usecase would exist somewhere.

@davidteren
Copy link

You could consider interactor

It's nice to use very lightweight interactors that call helper modules or classes which in turn are super easy to test in isolation.
Several interactors can be called by an organizer with each passing a context that will return the result and any relevant message.

@andreimaxim
Copy link

No, business logic is not scattered all across your app. The business logic should be in only one place: the models.

I've read Dave's Sustainable Rails book and I fully understand where he's coming from because I have worked at projects where everything is a mess (from actual SQL queries in the view to multiple layers of abstraction for doing simple things like listing the user's details) and every single time extracting "service objects" worked great. But I think that it worked great because:

  1. A lot of times code is written in haste, without much thought and ignoring best practices
  2. You are refactoring a piece of code that evolved over time and now you have a much better understanding of the domain and the real specifications

However, I don't think it's a very good idea to start with a service layer because Rails already provides a place where you should place all your domain logic and that is the app/models folder. I know that everybody thinks of "models" as "ActiveRecord models", but the files in the app/models folder are actually domain models and the fact that the data associated with some of the models is persisted at the database layer is just an implementation detail.

I think that a lot of developers forgot that code should be continuously refactored and we end up looking at a large piece of code and decide that we need to extract "service objects" or something similar. I was surprised when I read Jimmy Bogard's series of articles called "Domain-Driven Refactoring" and realized that refactoring, for me, gradually became this huge undertaking that involved changing a lot of code at once instead of refining it in small pieces (there are a lot of reasons for doing that, pressure from product managers to push a fix as soon as possible being one of them).

What's very interesting to me is that I've enjoyed your approach of keeping the app as vanilla Rails as possible. There's a lot you can you within that app/models folder and I think that creating a separate app/services folder feels like a (small) step outside of the Rails way of doing things.

Anyway, looking forward to see how the project evolves :-)

@joemasilotti
Copy link
Owner Author

What's very interesting to me is that I've enjoyed your approach of keeping the app as vanilla Rails as possible. There's a lot you can you within that app/models folder and I think that creating a separate app/services folder feels like a (small) step outside of the Rails way of doing things.

Thanks @andreimaxim, I needed to hear this.

The more service layer/objects I write the worse I feel about them. All I want to do is extract a DSL or parent object when I really want freedom to return whatever makes the most sense.

I also feel like I'm adding more and more code, and more and more indirection, to not accomplish much. Testing controllers via integration tests isn't hard these days. Yes, they are much slower than a unit level test. But I feel like with a sub 10 second test suite that's a trade off I'm willing to make.


I'm going to close this issue and accompanying PR #434 for now. Thanks to everyone who provided feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants