5

Would like to start off by saying, I am asking this as a purely academic exercise; I am quite happy to just continue to use the repository pattern.

I have heard "I put persistence methods in my business objects because when to persist was an integral part of the business logic" which I do not think is a valid reason, because that just means your business object needs a TimeToSave event

But one could argue, if you use the strategy pattern for the persistence methods, you are technically not including persistence logic in your domain layer. See example below:

class MyEntity
{
     private ISaveMyEntity _saver;
     public MyEntity(ISaveMyEntity saver) { _saver = saver; }

     public void Save()
     {
         _saver.Save(this);
     }

     //Real business methods
}

Including with a business object, its own persistence methods, could prove convenient. Furthermore, the repository pattern is often abused and ends up containing all the business logic instead of just CRUD, while the business object turns into DTOs; in my opinion, this is worse than a business object containing CRUD methods.

What could be the potential pitfalls of including access to persistence in a business object via the strategy pattern? Would use of the strategy pattern mitigate some of the problems commonly associated with storage code in a business object?

  • 1
    Since you have inverted control by using the strategy pattern, what is the benefit of having a reference to the persister, which probably should just be a function, inside the business object? Basically when you call Save you are just raising, as you say, a TimeToSave event. However, a smart persister is going to batch this request, cache it, and may in fact decide not to call it because some other state has changed. This could lead to confusing code where from the entity's point of view it has been persisted but it actually hasn't been. I'd be curious to see your intended usage pattern. – Aluan Haddad Apr 18 '17 at 23:29
  • 2
    By the way, I really like this question and I agree with you about the repository pattern. The business logic creep oft comes from premature optimization. – Aluan Haddad Apr 18 '17 at 23:35
  • 1
    Aluan, I like the points in your first comment, I am not sure why you did not post it as an answer? Yes, I think it is partly premature optimization, but also just plan not understanding how to separate business logic and what it constitutes. – TheCatWhisperer Apr 19 '17 at 15:34
  • The main reason I didn't write it as an answer is that I want to see an example use case before coming to a conclusion. – Aluan Haddad Apr 20 '17 at 2:44
  • 1
    Alaun, this is an academic discussion. I always just use an external repository ;) – TheCatWhisperer Apr 20 '17 at 14:28
8

You've got value objects representing your business logic, containing business rules. Now you are adding a mechanism to persist them. Fair enough. How about you add a mechanism to read a database and construct them from it? Perhaps a static Load method taking IGetMyEntity instance as a parameter?

While you are at it, maybe it is the best to add a few more methods:

  • RenderAsHtml,
  • ToString,
  • ToJson,
  • ToXml.

You see where I am going with this? It all starts with a simple Save method, but a few commits later your objects suddenly know everything, have low cohesion, everything is crammed up in a single place and impossible to reuse without dragging the rest of the unwanted boilerplate with it.

The repository pattern became very popular because it scales well. If the object you are talking about is really supposed to be an in-memory representation of a business rule, it is NOT okay to include CRUD methods as a part of its API.

To expand on your opinion about DTOs, when you are doing some operation, where do you generally need a validation of business rules? Is it on reads? Most likely not. You may prohibit access to some resource, but it's more of an authorization issue rather than a business rule.

The place where you are likely to enforce business rules is when data comes into your system (be it completely new or coming through an update). On that occasion you need to check the operation altering state of your system is valid, ie. may be performed. For this you should use value objects or entities enforcing the constraints. Repository really is not a place to contain business logic. The goal of a repository is to abstract persistence of objects, not to enforce business rules. The objects passed to the repository MUST make sure they are valid themselves before being passed to a repository for further processing.

Also it is better not to look at a repository as a class knowing all CRUD operations, but rather like a layer within your application. When your application grows it's likely you are going to need multiple representation of an entity on reads. Should you have a repository as a single class having multiple Get* methods does not feel right and the class could become quite big and difficult to maintain soon - coming from a personal experience.

  • wooooaaaa there! No one said anything about static methods in business objects! Your potty mouth is not appreciated ;) – TheCatWhisperer Apr 18 '17 at 20:33
  • 1
    @TheCatWhisperer I am sorry if it came out that way. That was definitely not the intention. I was trying to state the obvious. Because you had mentioned the CRUD operations, when you want to actually create the business object using a persistence layer it makes no sense to be done through a constructor, then you would have the entity already and would not need the persistence. Hence the mention of a static method. – Andy Apr 19 '17 at 6:12
  • David, no worries, I was being somewhat facetious. However, I could new an object up, then call the instance Load method by passing it the Id I want and then the object would hydrate itself. – TheCatWhisperer Apr 19 '17 at 15:28
6

There are many reasons why DTOs are used for persistence, but perhaps the nastiest one is database transactions. (i.e. where multiple steps are needed and you can't just perform a single neat UPDATE, INSERT, DELETE, etc.)

Some of the basic requirements for transactions are as follows:

  • Multiple steps can't easily be performed in a single neat SQL statement
  • No COMMIT happens until all steps have succeeded
  • If a step fails, a ROLLBACK happens to undo the entire transaction and leave the database in its previous state - no partial updates or orphaned records.

Consider the business objects in your application:

  • Business objects support the behaviour of the application
  • Their lifetime will necessarily be linked to their behaviour
  • They may contain state which shouldn't be reflected in the database

Now consider the scenario where all of your data is pulled straight into your business objects from the database - how do you perform a complex transaction which might require multiple queries and UPDATE/INSERT/DELETE statements?

  • Where do you put the transaction logic? If it involves multiple business objects then it clearly doesn't belong in any single one of those objects.
  • Related to the previous bullet point, which object is responsible for commit or rollback when a Commit or Rollback affects the data from multiple business objects?
  • How do you handle transactions for relationships between business objects which are represented as Many-to-Many in the database? (e.g. Student-Teacher)
  • What happens if when the transaction is cancelled and needs to rollback? The domain objects are tied to your behaviour so you can't just throw them away, but they contain the data from the failed transaction.
  • What if the data which had been cached in the domain object ended up being out-of-sync with the database?

The key problem is this: Business objects are not transitory. By adding data into them, their data implicitly becomes part of their state because you've cached that data, and the lifetime of the cache is the same as the lifetime of the business object.


Why do DTOs solve this?

  1. If a business object uses a DTO for its data, then the data isn't part of its state. Similarly, a business object might want data from multiple DTOs and that is also fine.
  2. DTOs are transitory objects by their nature; their lifetime is not tied to the behaviour of your application.
  3. Your business objects don't need to care about the lifetime of a DTO for its data; if one gets thrown away then all you need is a new query into the database to get back a shiny, fresh, clean new one.
  4. You can write a transaction with DTOs without messing about with the state of your domain objects, because your transaction data is not part of any domain object.
  5. If the transaction fails, you throw away all of the DTOs (those are transitory objects anyway and would also have been thrown away upon success). Your business objects don't need to worry about being left in a potentially invalid state.
  6. Your application can choose which data to cache based on whether there are performance benefits from caching; instead of 'all' data being cached by default.
  7. You don't need to implement separate cache invalidation or re-synchronisation with the database for every single one of your business objects.
  8. When data isn't cached by your domain objects, any behaviour related to that data will always be queried directly from the database, so you're assured to be working against the latest data.
  9. Lastly, when you need to write transaction logic involving multiple entities, you don't need to worry about which of your business objects it belongs to because the business objects are no longer tied to the data.

Conclusion - once the lifetime of your behavioural domain objects determines the lifetime of the objects which read and write to your database, it introduces issues of code organisation, transaction rollbacks, cache invalidation and data synchronisation.

While it can seem "clean" when your diagrams are filled with well-defined business objects which each have a strong identity, easily relatable back to the problem domain, and which encapsulate all of their own data and behaviour in a single place, the reality of multi-object interactions and the complexities of managing persistent data in an external database is far from clean.

  • This is a good answer, but it is not a good answer to this question. You're answering, "Why should I not abuse the active record pattern?" There is no reason to assume the business object represents an active record just because it possesses a save method. Also, there is no reason you cannot pass a transaction token to the save method. None the less, I have never seen the active record pattern not abused in production code, so I still think your answer is useful. – TheCatWhisperer Apr 19 '17 at 15:26
3

Seems to me that this could potentially make the objects inconvenient to use if no persistence was needed. To unit test, I would have to mock the persistence interfaces; with an event based system, I can just choose not to subscribe to the SaveMe event.

Another potential problem is that doing things this way could make things potentially difficult in the instance of doing efficient batch CRUD operations.

  • 1
    This is one reason why in my comment, I mentioned using a straight-up function for the persister, it is easy to stub out: new Entity(_ => {}). It doesn't solve the problem but it makes testing easier. – Aluan Haddad Apr 19 '17 at 0:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.