Ask AyendeAggregates and repositories
With regards to my recommendation to avoid the repository, Stan asks:
You returned to explain why repository pattern is evil. Just interesting to know what are you doing when you need in your model to access another aggregate. Do you reference NH from your model? I prefer to leave my model POCOed and wrap DB calls by repository pattern. Sleeping good with it.
This is an interesting question. There are several ways to answer that. To start with, assuming that we are using DDD model (which the usage of a repository would imply), you don’t have references between aggregates.
But let us assume that we somehow need that, Stan seems to suggest something like this proposed solution:
public class Person { public static readonly DateTime ImportantDate; public BirthPlace BirthPlace { get; set; } public DateTime BirthDate { get; private set; } public void CorrectBirthDate(IRepository<BirthPlace> birthPlaces, DateTime date) { if (BirthPlace != null && date < ImportantDate && BirthPlace.IsSpecial) { BirthPlace = birthPlaces.GetForDate(date); } } }
Here we have a business rule that states that this is required.
But do we actually need a repository here? What if we just said that whoever calls us need to provide us with a way to get the birth place by date?
public void CorrectBirthDate( Func<DateTime, BirthPlace> getBirthPlaceFordate, DateTime date) { if (BirthPlace != null && date < ImportantDate && BirthPlace.IsSpecial) { BirthPlace = getBirthPlaceFordate(date); } }
This can be done with a simple delegate, no need to introduce a heavy weight abstraction. This is a local solution for a local problem. It keeps the database out from your entities and more importantly, it allows you to actually craft the appropriate response to this at the time of the call.
More posts in "Ask Ayende" series:
- (28 Feb 2012) Aggregates and repositories
- (31 Jan 2012) What about the QA env?
- (25 Jan 2012) Handling filtering
- (19 Jan 2012) Life without repositories, are they worth living?
- (17 Jan 2012) Repository for abstracting multiple data sources?
Comments
Ayende what do you mean by "you don’t have references between aggregates". Isn't that allowed to have one class in aggregate referencing some other Aggregate Root? Do you mean that those Aggregates are Islands?
The characteristics of an aggregate is that it is an isolated context and isolated model.
If you need access to another aggregate you use context maps or other constructs depending on the scenario.
AG, I meant, you don't hold a direct reference to another aggregate. Order doesn't have a Customer property of type Customer. You explicitly have boundaries between aggregates.
Patrik, aren't you mistaking bounded context with agregate? I believe agregate is normal entity, that encapsulates access to other entities or values, that are in this agregate.
It is normal to have references between agregates.
Ayende. Didn't you just replace one (specific) abstraction with different (general) abstraction?
There is still abstraction. But you moved from specific class and it's implementation, to compiler-helped one, where you only dont need to write so much code.
So, are you saying, abstractions are OK as long as they are general and don't need lots of code?
Why would it be bad to create a method "bool NeedsToCorrectTheBirthPlace" ?
@Falhar with ayendes aproach you don't need to create the whole abstraction of Repositories just to solve this one local problem.
Falhar, Not really. I didn't create any new abstraction, I simply inversed the relationship. I still think this is bad code, but that is at least a step in showing that you don't need a repository for this, and it makes calling this code very easy
Roland. This is reason to have IRepository abstraction right from the begining. And this is not only reasons. There are many reasons.
And I don't like this change, because it changes the meaning of this rule. In example with repository, you can control how do you get the correct birthplace. In the other one, you miss that control. And I believe this is a possible bug.
@Ayende. It is still an abstraction. Except creating interface and implementation, you use generic code and help of compiler to get less code, with less control.
Wouldn't this require passing along Func<DateTime, BirthPlace> getBirthPlaceFordate whenever you wish to use CorrectBirthDate in any kind of more complex situation?
I am really curious about your proposal since it has also been troubling me in most of my projects. How much control do you give the the 'model' or something similar? Do you allow it to load objects as it demands it or do you give it everything from the outside.
@Ayende: Is it possible to convince you to do a little more blogging about that issue? Pretty please :)
P.S. These captchas can be pretty horrific ... I can not read what is written there!!
Falhar, Here is how I think about it: an interface (IRepository in this case) is representing a group of methods. However the example operation (CorrectBirthDate) only requires one of the methods on this interface. So why burden it by having it depend on an interface when all it needs is one method?
The beauty of working on a language which supports delegates is that you have a smaller abstraction available: a delegate instead of an interface. The abstraction wasn't removed from this example, it was simply replaced by a smaller one. This reduces overall coupling in the code while keeping high cohesion.
@Joseph: How do you ensure, that correct method is passed in?
With repository, you ensure 2 things: You select from all BirthPlaces, that exist and you select the correct one.
Also, the first example is wrong. You would either need to pass concrete interface, like IBirthPlaceRepository. Or change the called method to something more generic, like birthPlaces.Query().First(x=>x.Date==date) . Where Query allows you to use LINQ to (whatever DB backend you use).
I like the second solution, because, you create only ONE abstraction for whole application. That is generic IRepository<TEntity>, with Save(..), Delete() and Query() . No need to create concrete repository for each entity.
You can also pass, IQueryable<BirthPlace>, which I would preffer over simple delegate.
I have to admit having a delegate/lambda there seems very... ugly. It allows for no re-use (although I guess that begs the question, should something be reusable from the start in case it needs to be reused, or should it be localized and made reusable if a situation arises where it has to be reused) and I just feel it lets the consuming code know too much. A hypothetical controller in this scenario would be directly calling and loading all of these objects? I'm not sure.
Hi guys,
isn't this again another discussion which logic belongs to the aggregate itself and which logic doesn't ?
Maybe it makes sense to further investigate if the logic does even belong to Person. This logic has a dependency (the repository), so I ask myself the question if it makes sense that it is on Person rather than somewhere else in a piece of code which "works" on a Person object.
@Falhar about "@Joseph: How do you ensure, that correct method is passed in? With repository, you ensure 2 things: You select from all BirthPlaces, that exist and you select the correct one."
What do you mean by correct method? The method gets a date and returns BirthPlace. Everything else is implementation details. And the two things you mention are also implementation details.
The purpose of the delegate and repository is that the actual process "getBirthPlaceFordate" is not in the method but abstracted away. If you you use IQueryable you change the logic of the method and that is not the purpose of this post.
@WayneM Why do you think that it doesn't allow reuse of code? It just leaves the implementation details away. If you want to make the code of "getBirthPlaceFordate" reusable that's up to you to decide.
Wayne, it doesn't not allow for reuse. Ayende never said a lambda, it's just a delegate. As a delegate, it could be any method anywhere which fulfills the contract. It could be a static helper, a lambda, or even a repository method.
So in the case the caller might have a reference to IRepository<BirthPlace> with a GetForDate method. When the caller calls CorrectBirthDate, it simply passes in the method. For instance, it might look like this:
{ IRepository<BirthPlace> birthPlaces = .... Person person = .... person.CorrectBirthDate(birthPlaces.GetForDate, someDateTime); }
So all we've done here is removed the dependency on the repository from the CorrectBirthDate method. Instead it just has a dependency on the single function, which is still fulfilled by the repository.
The main benefit is a much looser coupling in the code. But you still have high cohesion.
@Apostol: No. This is not implementation detail. This is doman detail.
With repository, the method says: - if date changed - from all BirthPlaces - select the one by date
With delagate, the method says: -if date changed -get BirthPlace with date parametter
You cannot enforce, that the delegate passed will have correct domain logic. And slecting correct BirthPlace IS domain logic. Not implementation detail.
This whole thing asumes, the one who will use this code, will not change the meaning of IRepository. When I have repository, I know I work over all persisted entities with specific domain logic.
This refactoring changed the meaning of the method.
@Falhar But you cannot Enforce The Repository to have the correct domain logic either.
The meaning of the repository in this case is to get the proper BirthPlace by date. The meaning of the delegate is the same. With the repository you just include a unnecessary abstraction which makes the code harder to use.
You basically trade more lines of code for a feeling that the code is properly structured. You can't even enforce that the code would be properly structured.
If you want the code to be properly structured - make a Coding Style document and follow it. Don't write more code.
@Apostol: But with Repository, the logic is still a domain logic. Or at least, can be enforced as such. With delegate, I can give it any method. Not only the one containing domain logic. And noone is gona ensure me, that the correct method will be used.
If someone else will want to change BirthDate, then it is not obvious to know what kind of method they should pass in. With Repository, this is obvious. Because there is only one in whole application. (either one specific, or one generic)
Falhar, You mean, in the same sense that I can give you CheatingRepository : IRepository ?
@Ayende: But it is easier to ensure correct repository is used, than ensuring correct method is passed as parameter.
@Falhar: Why do you think it would be easier?
person.CorrectBirthDate(myRepository);
vs.
person.CorrectBirthDate(MyMethod);
They both require I go somewhere else.. and that assumes that MyMethod isn't so simple that I can just inline it. In fact myRepository is worse because I need to go find the actual implementor. If we're talking typical IoC this means back tracking it based on my conventions.
Also, lets look at it from point of some other developer, who needs to use it.
void CorrectBirthDate(IRepository<BirthPlace> birthPlaces, DateTime date)
"So, to change date, I need to give it correct date and this repository. Sure." Because Repository is general term and developer knows what it means, they can easily tell what the method wants and how to get the repository.
void CorrectBirthDate(Func<DateTime, BirthPlace> getBirthPlaceFordate, DateTime date)
"So, to change date, I need to give it date and .. how do I get BirthPlaceForDate ??" This needs developer to look for more information, probably by asking someone or looking at other usages of this method, so they can call this method.
Falher, That's a fallacy. You shouldn't be designing your application around assumptions about the stupidity of your developers. You should be designing based on solid principals.
I see that depending from person onto the birthplaces repository is bad and depending on a method might be better. But would that not just pull the birthplacesrepository up a level? What does the caller of this method look like, does it not need that repository?
@Shane: No, no, no. Where do I get the MyMethod? How do I know what should it implement? Didn't someone already implement it? Is the implementation correct? It is not MyMethod, but IRepository<BirthPlace>.GetForDate , no other method can be passed, othervise you change the domain logic.
And there should already be infrastructure to create concrete implementation of IRepository from anywhere where you call it from. Just like Ayende's ExecuteCommand from few articles back.
@Joseph: That is not idiocy. That is when developer, which didn't wrote the code, tries to use that code. Which is common practice. Unless you are only one doing whole project.
Generic delagates are too general as abstraction. Noone will know WHAT should be passed into it, unless some other source tells them.
@Falhar, I think you are spot on with your last comment. I try to use types to provide more information to me and to developers around me. For that reason I always create Id-classes for objects instead of using ints. I also create LatLon structs instead of using Point or Vector.
Moreover, I don't like passing around repositories like in the example either.. Repositories are infrastructure and I get a bad feeling when I pass infrastructure around. A couple of options:
I would probably use a CorrectBirthDateService for this; the repository gets injected in the service constructor and the Person object gets passed as an argument to CorrectBirthDate.
If there was a direct mapping from DateTime to BirthPlace I might have gone with a variant of Ayende's solution: passing IDictionary<DateTime, BirthPlace> instead of a Func<-,->. Bit less generic though, but that is what I would have come up with.
For me, part of the reason to use static typing is to remind (rather than force) myself of what objects are needed so that I can go look for the information I need.
@Markus
"I would probably use a CorrectBirthDateService for this; the repository gets injected in the service constructor and the Person object gets passed as an argument to CorrectBirthDate."
That would be my first instinct but the whole point of these posts is to limit the abstractions, not add additional ones ;-) I think the whole underlying point here is that a
CorrectBirthDateService
is overkill, having a separate repository is overkill.@Wayne M Where is the abstraction? I never mentioned any ICorrectBirthDateService. If anything, it is adding (perhaps overkill) specifics, not abstractions.
@Joseph, that's a joke. I always design my application around the "stupidity" of future developers, because 90% of the time, that stupid developer is me.
I would never want to come back to this example code after 6 months, especially if the getBirthPlaceForDate logic was different in each place it was used.
If the Person class really needs birth places to do its job, why not make it member data (and include birth places in the OR/M mappings)?
In cases where you need to cross aggregate boundaries, I usually use an extension method. This is useful as the entity is relatively persistence ignorant but has the capability to execute business rules using data it needs. Going with this approach means that your root entities don't reference each other directly, but when they need each other, the code to retrieve the data isn't duplicated.
The downside with this approach is the abstraction which Ayende is trying to hammer away. The approach cited in this example means the developer is in full control. That being said, I prefer the extension approach. If it's common practice with a specific development team, I think it works quite well and keeps your code neat. I agree with the folk saying that the delegate approach would create confusion in future.
If it's not clear what I'm suggesting, here is an extension method on a user entity to grab the user's orders. If you're using DI, you could use your object factory to instantiate the repository. Testing nuts will tell you that this approach isn't a good one, however, I think the overhead of mocking the repository in your test setup is worth the gain in simplicity when actually consuming the code.
public static class UserExtensions { public static IEnumerable<Orders> GetOrders(this User user) { return new OrderRepository().GetOrders(user.Id); } }
@Fahlar With repository, you ensure 2 things: You select from all BirthPlaces, that exist and you select the correct one.
You don't ensure anything by using repository. You only have interface in method signature. You don't know what is passed! So there is no difference between delegate and ISomeRepository.
Business rules being expected to live within the boundary of an Aggregate is a fallacy in my opinion. Business rules are conjured up in the minds of humans and can span any number of entities and can change at any time to use altogether new entities and/or additional attributes. This is why in my opinion DDD is not a valid approach. I'm sure there are many exceptional workarounds for this in DDD but it is fundamentally flawed in this regard.
As for access to a Repository (or Store or DAO) instance. It is best to use Injection (IoC) of the Repository which is then controlled from a central IoC binding for your module. Specifically for the case of repository like functionality - the Interface/Class approach is better than a Delegate function due to many reasons: first of many methods might require to interact with the same repository (if 2 entities are used in one business rule, likely it will be used together under more rules in the same service), second: the Delegate's function signature will cause a nasty ripple effect as each caller tries to fulfil up the call-chain.
How about mathematical proof that both approaches are complete dual of each other and we can prove it by bringing it into pull/push analogy. Meanings you put into those are totally human related and its all about your team and your target audience.
This would means both ways do provide similar guarantees and do provide similar modularity. Please go ahead and prove otherwise.
Jimmy bogards presentation covers this very well!
https://github.com/jbogard/presentations/tree/master/WickedDomainModels
Daniel
Imagine that in the example you have not one but ten delegates for performing various queries How is such set of delegates called? An Interface! But don't you ever try to name it IRepository - it's bad practice. Name it something like ISetOfEntityOperations...
Rafal, If you had 10 operations going on, you are doing it wrong.
@Sla
Spot on!
The delegate solution is no more and no less abstract than the IRepository solution, and it's no more and no less enforceable.
The objective was to show how a data accessing double dispatch can be made, without an unnecessary IRepository and without injecting an "ISession" into the method. The delegate itself can be as loosy lambdaish or as strongly typed and as testable as you'd like.
I have to agree with Rafal on this one, using delegates in this case seem very wrong to me, it makes the code unclear and confusing for the user.
I don't see how
public void CorrectBirthDate(IRepository<BirthPlace> birthPlaces, DateTime date)
is any less confusing than
public void CorrectBirthDate(Func<DateTime, BirthPlace> getBirthPlaceFordate, DateTime date)
If anything, the latter is more explicit.
@Falhar,
Does it make you more comfortable with this shift if instead of this signature:
void CorrectBirthDate(Func{DateTime, BirthPlace} getBirthPlaceFordate, DateTime date);
we have something like this one:
delegate BirthPlace BirthPlaceRetriever(DateTime d);
void CorrectBirthDate(BirthPlaceRetriever getBirthPlaceFordate, DateTime date);
Structurally it is identical and will work identically as far as the compiler is concerned (i.e. you can pass repository method group, lambda, etc.) but is converying the same semantics as the interface would do, except in a much more granular fashion. .
Using delegates in this way actually reduces clarity. You seem to have a delegate fetish Ayende as this kind of thing is all over the RavenDB Codebase. I personally prefer classes for most things (there are exceptions), much easier to track and much more explicit.
The reason both the original and proposed solutions seems wrong is that in this case you would probably be better of just moving the birthplace resolution logic out of this class.
@Ayende, thanks for an answer. I was out here for some weeks and only now saw this blogpost. I think I got your point but cant agree with you. I dont like this solution. Its unclear what method should be passed and where to look for it. It smells bad. In my case I do not pass repository into a method. Following given example I inject IRepository<BirthPlace> into Person in NH load event listener when it's fetched. It will be very ugly starting to register loads of delegates in DI container aspecialy when some of them will have same signature. Also it will be much harder to automate registration proccess of delegates than repositories. Injecting some sort of IGetBirthPlaceFordateQuery instead of repository seems better than injecting delegate. I'll think about it.
I do not understand the business requirement here, but we use domain events (see Udi Dahan) for communication between aggregates and do not need any dependencies or delegates in our aggregates and entities.
Comment preview