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

Add guide for where to put service classes #652

Merged
merged 1 commit into from May 13, 2022
Merged

Conversation

sallyhall
Copy link
Contributor

Distilled from a slack conversation about where to put service classes

@JoelQ
Copy link
Contributor

JoelQ commented Apr 22, 2022

Potentially relevant prior art:

rails/README.md Outdated
@@ -15,6 +15,9 @@
- Order i18n translations alphabetically by key name.
- Order model contents: constants, macros, public methods, private methods.
- Put application-wide partials in the [`app/views/application`] directory.
- Put classes (Service Objects/Interactors/et al) that may later be extracted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for distilling this! I have a few thoughts:

  • Should this be two bullets? (One about lib/ and one about app/jobs)
  • Maybe something like "Use lb/ for code that will later be extracted to a gem" to describe lib? I think it could be anything generic, it's hard to make a finite list. My opinion is that lib/ shouldn't have app-specific code.

Copy link

@thiagoa thiagoa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. About jobs, I think they should, ideally, be thin wrappers over existing services -- but that is an optimization that can be gradually applied as needed. I've always found a tension between moving code between services and jobs, and in the end I think that distinction is not so important. Code that runs synchronously should also be able to run async.

Also, when the app reaches a certain complexity threshold, I prefer extracting concepts and trashing most of the extra non-MVC categorization -- where some of these guidelines would not apply. Otherwise, I think the "Rails way" of structuring code works.

With the app/services, app/jobs, etc, arrangement you're always forced to put things in buckets, and app/models is usually the trash folder for code that doesn't have a category.

If you structure code in a certain way, its category becomes less important. For example, it's clear that this represents a service: app/concepts/sales_tax/calculate_sales_tax, and anything we include in concepts/sales_tax wouldn't need an explicit bucket.

Sorry for the tangent here, I think that's a different discussion 🙂

@sallyhall sallyhall merged commit 4cbcfdc into main May 13, 2022
@sallyhall sallyhall deleted the sh-service-classes branch May 13, 2022 18:37
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 this pull request may close these issues.

None yet

5 participants