phalt / django-api-domains

A pragmatic styleguide for Django API Projects
https://phalt.github.io/django-api-domains/
MIT License
705 stars 62 forks source link

Clarification on Model / Service business logic ownership #1

Closed phalt closed 5 years ago

phalt commented 5 years ago

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:

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()
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:

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()
class BookService:
    @staticmethod
    def get_book(id: uuid.UUID) -> BookTuple:
        book = BookModel.objecets.get(id=id)
        return BookTuple(
            author=book.author
        )
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 commented 5 years ago

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 commented 5 years ago

So to help clear up the language here:

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

wearp commented 5 years ago

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 commented 5 years ago

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