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

Clarification on Model / Service business logic ownership #1

Closed
phalt opened this issue Mar 27, 2019 · 4 comments
Closed

Clarification on Model / Service business logic ownership #1

phalt opened this issue Mar 27, 2019 · 4 comments
Projects

Comments

@phalt
Copy link
Owner

phalt commented Mar 27, 2019

I've had people asking for more clarification on what business logic should live where. "Should I put this method in the model or the service layer?"

A common anti-pattern is the anemic data model, where Models are so basic they basically set and get data. I would like to avoid that, and I didn't plan for DADs to promote that. My main goal in the separation was drawing a line between presentation/service-level functionality and data storage functionality

The current DAD (Django Api Domain) version rules that Models should not have any complex business logic, but they can do simple thing like provide computed values, like so:

class Book(models.Model):
    author_name = models.CharField(max_length=256)
    book_name = models.CharField(max_length=256)

    @property
    def book_and_author_name(self):
        # A simple property method
        return f'{self.book_name} - {self.author_name}'

The Services should handle more complex business logic across multiple Models, or multiple domains.

I think we need to add some more examples of what type of logic should live where, and I want to allow suggestions from the community.

Here is my current feeling on the matter:

Models

Models should handle the following business logic:

  • Simple computed values from the Model's attributes
  • Common filter methods:
class Book(models.Model):
    @classmethod
    def get_available(cls) -> List[Book]:
        return Book.objects.filter(on_loan=False, for_sale=True)

>>> available_books = Book.get_available()
  • Wrapping up common update or save actions on the model:
class Book(models.Model):
    def update_days_on_loan(self) -> None:
        self.days_on_loan = self._days_on_loan + 1
        self.save()

>>> Book.update_days_on_loan()

Services

Services should be putting together service-level or presentation logic that might spread across the whole domain or many domains. For example:

  • Combining separate actions into a single action:
class LoanService:
    @staticmethod
    def rent_book(*, book: Book, user: User) -> None:
        book.set_on_loan(user)
        book.reset_days_on_loan_count()
        user.update_books_rented()
  • Translate a Model's model object into serialisable objects:
class BookService:
    @staticmethod
    def get_book(id: uuid.UUID) -> BookTuple:
        book = BookModel.objecets.get(id=id)
        return BookTuple(
            author=book.author
        )
  • Translating input data into data we want to store on a model:
class BookService
    @staticmethod
    def create_book(
        author_first_name: str, 
        author_surname: str, 
        name: str
    ) -> None:
        Book.objects.create(
            author=f'{author_first_name} {author_surname}',
            book_name=name,
        )

Is this level of clarification sufficient?
Should the bar be moved more in one direction or the other?

Feedback please.

@iemre
Copy link

iemre commented Mar 28, 2019

This makes sense. I prefer to phrase the same thing in the following way.

Model layer must own the information i) Simple information from data fields e.g. amount field in Order class ii) More complex information computed from simple data fields e.g. think of a method called get_discounted_amount that subtracts the simple data field discount from amount and gives you the discounted amount.

Service / business logic layer must take care of transactions and coordination to make a certain thing happen e.g. save the Order and send an email to the customer. To achieve this the service layer uses the information coming from the model layer.

I have seen previously methods like get_discounted_amount placed in the service layer because it is "business logic", which it probably is. There is definitely logic there, in the sense that it is computing something. But it is information about the model, on its own this method is not making anything happen. I prefer to put these methods that give us information in the model layer.

@phalt
Copy link
Owner Author

phalt commented Mar 28, 2019

So to help clear up the language here:

  • Models control logic around "information" - presenting data, simple computation.
  • Service control co-ordination and transaction logic - collating information from Models, ensuring actions happen together.

I think the term "business logic" is too abstract for this guide, so I think we should drop it.

@phalt phalt added this to To do in VERSION 1.1 via automation Mar 28, 2019
@phalt phalt moved this from To do to In progress in VERSION 1.1 Mar 28, 2019
@wearp
Copy link

wearp commented Apr 2, 2019

Agree with @phalt - "business logic" is too abstract for this. It becomes loaded for meaningful discussion.

I have a preference for creating boundaries around natural sets of entities. A timely example is ConceptGroup and SubConceptContainer. There is a business rule which says a ConceptGroup must have at least one SubConceptContainer. The ConceptGroup entity seems naturally responsible for enforcing this invariant, not the SubConceptContainer nor a client such as a service layer. Enforcing at this level means the model can be reused (management commands, migrations, etc.) without having to replicate business logic or having to use the service layer.

Where these sets of entities interact, this could be done through a service layer. This layer would be responsible for co-ordination and doing data munging to comply with the model layer's api (there's quite a bit of this).

@wearp
Copy link

wearp commented Apr 2, 2019

Also, without going crazy, I think we should just experiment. In combination with best practice, we'll find something that suits us.

@phalt phalt closed this as completed in #2 Apr 8, 2019
VERSION 1.1 automation moved this from In progress to Done Apr 8, 2019
@phalt phalt pinned this issue Feb 9, 2020
@phalt phalt unpinned this issue Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
VERSION 1.1
  
Done
Development

No branches or pull requests

3 participants