DDD does this (database)logic belong in the model

Question

I'm trying to figure out where to put business logic and why and where performance fits in. Im trying to get away from the fat logic-filled repositories that we produce a lot in my company.

So i'm trying to refactor this method DDD style:

var doesCollide = _bookingRepository.HasCollisionWithOtherBookings(bookingEntity, newEnd);

What i think i want is the following:

var doesCollide = booking.CollidesWithOtherBookings(allBookings, newEnd);

The internal logic is just doing some classic date mathematics. I still need to load allBookings from the database.

var allBookings = context.bookings.ToList();
var doesCollide = booking.CollidesWithOtherBookings(allBookings, newEnd);

But now I'm thinking, ALL of the bookings? I can improve performance a lot by just adding some simple conditionals to my query.

var allBookings = context.bookings.Where(x => x.Start < newEnd && booking.Start < x.End && x.Id != booking.Id).ToList();
var doesCollide = booking.CollidesWithOtherBookings(allBookings, newEnd);

Great. But i literally just duplicated the exact logic that i attempted to hide in CollidesWithOtherBookings. What does that even mean? Do i have to test this new query somehow?

Im not sure what im even testing here. The datetime logic is easy to mess up, so i should probably test it. If i test Booking.CollidesWithotherBookings thats great, but in my actual application i could have a bug since i can mess up in the where clause.

Answer 1

I'm not a C# expert, I'm not a MVC or MVP evangelist (I dislike these design patterns as they are too simple to effectively model complex systems), and I might already be an old fogey in the eyes of some people - but one thing I've been taught when I was still a whippersnapper, that I've complied with all my professional career, and that hasn't changed up to today AFAIK is this one rule:

Never write code to solve a problem that the database could have solved for you.
Just leave it to the database.

Why? That's what databases are for. If they were just for storing data, a simple linear data blob would have worked as well. Why do databases keep data in complex data structures, why do they build up indices, and why do they offer filtering and sorting? Because that's what they were intended for, that's what they were optimized for, and that's what they are good at, probably better than any code you could ever write.

(And this was the nice version of the rule. When I was taught that rule, the last sentence was more like "Just leave it to the database, STUPID!")

Answer 2

Theoretically speaking, the collision check belongs in the model and not in the repository.

Practically speaking, there are more considerations to take into account, such as the complexity of the collision check and efficiency concerns.

If your collision check is easily expressed as a readable WHERE clause in an SQL query, for example your example of time overlap with other bookings, then it would be foolish to discard the filtering capabilities of the database or duplicate the check as you effectively did in the last snippet. In such a case, just build the right query in your repository class and let the database do the work.

If the collision check is more complicated, for example because there must be a variable time gap between bookings depending on the locations involved (to account for travel time), then it is best to do the actual collision check in the software. The repository could offer a method to retrieve all bookings within a certain time frame to reduce the data read from the database to those records that might potentially cause a collision.

Answer 3

You already have two great ideas:

1. Let the objects have domain-centric methods. Like booking.Collides() or similar.
2. Why duplicate logic which the database can also do for you.

The problem you're having is not because these ideas are wrong, but the pattern and/or the framework/library you're trying to follow doesn't seem to allow you to do it.

The easy solution is to just not use those. Also, don't worry, you are obviously more domain-driven if your domain-objects have domain-specific methods.

Answer 4

I'm trying to figure out where to put business logic and why and where performance fits in.

The general term for the problem you are trying to solve: set validation.

The easy answer, and most familiar -- relational databases are really good at set validation. If the entire set fits in a single database, and if you can express the uniqueness constraint in a way the database understands, then that will get you a lot of the way there.

But you give up "all of the logic is in the domain model".

Another possibility for uniqueness is to make the constraint that needs to be satisfied part of the key that is used to extract models from the database. For example, if you want to ensure that every email address is associated with no more than one model, then use a key that depends on that email address. Models that try to share the same address will share the same key, and you can detect "empty" vs "not-empty" sets without having to bring down the entire table.

But if that doesn't work, then you need to be thinking about locking your entire set of models when you make a change. Whether that is because your "aggregate" is a representation of the entire set, or because you use a table lock to prevent conflicting writes, is a matter of circumstances (ie: "it depends").

One thing to be sure that is well understood is the cost of an error; if duplicate errors are easily detected and corrected, then it may make sense to settle for a "best effort" strategy in preventing duplicates, rather than treating it as an absolute.

Answer 5

You can achieve a best-of-both-worlds solution by explicitly modeling the concepts at hand. In order to do so we need to re-frame the problem a little bit: You do not have a Booking entity that may or may not collide with other Booking entities given a newEnd, rather you have an ExtendBookingRequest (VO) that does or does not overlap with other Booking entities. For example:

class ExtendBookingRequest
{
    bool DoesCollideWithOtherBookings;

    DateTime newEnd;
}

This can be accepted or rejected as appropriate. Yes, all it is doing is acting as a domain container for the values you intend to use as part of enforcing your invariants, but it brings the benefit of keeping the decision of whether or not to reject or accept the request in your domain (my assumption here is that your current method being invoked and enforced at the service level).