+972 52-548-6969
Taking a look at S#arp Lite, Part II–the domain
This is a review of the S#arp Lite project, the version from Nov 4, 2011.
In my previous post, I looked at the general structure, but not much more. In this one, we are going to focus on the Domain project.
We start with the actual domain:
I have only few comments about this sort of model:
- This is a pure CRUD model, which is good, since it is simple and easy to understand, but one does wonder where the actual business logic of the system is. It might be that there isn’t any (we are talking about a sample app, after all).
- The few methods that are there are also about data (in this case, aggregation, and Order.GetTotal() will trigger a lazy loaded query when called, which might be a surprise to the caller.
- Probably the worst point of this object model is that it is highly connected, which encourages people to try to walk the object graphs where they should issue a separate query instead.
Next, let us look at the queries. We have seen one example where NHibernate low level API was hidden behind an interface, but that was explicitly called out as rare. So how does this get handled on a regular basis?
Hm… I have some issues here with regards to the naming. I don’t like the “Find” vs. “Query” naming. I would use WhereXyz to add a filter and SelectXyz to add a transformation. It would read better when writing Linq queries, but that is about it for the domain.
One thing that I haven’t touched so far is the entities base class:
And its parent:
I strongly support the notion of ComparableObject, this is recommended when you use NHibernate. But what is it about GetTypeSpecificSignatureProperties? What it actually does is select all the properties that has the [DomainSignature] attribute. But what would you want something like that?
Looking at the code, the Customer.FirstName and Customer.LastName have this attribute, looking at the code, I really can’t understand what went on here. This seems to be selected specifically to create hard to understand and debug bugs.
Why do I say that? The ComparableObject uses properties marked with [DomainSignature] for the GetHashCode() calculation. What this means is that if you change the customer name you change its hash code value. This hash code value is used for, among other things, finding the entity in the unit of work, so changing the customer name can cause NHibernate to loose track of it and behave in some really strange ways.
This is also violating one of the core principals of entities:
A thing with distinct and independent existence.
In other words, an entity doesn’t exists because of the particular values that are there for the first and last names. If those change, the customer doesn’t change. It is the same as saying that by changing the shirt I wear, I becomes a completely different person.
Domain Signature is something that I am completely opposed, not only for the implementation problems, but because it has no meaning when you start to consider what an entity is.
Next, we are going to explore tasks…
Comments
Hi Ayende, I agree with you about DomainSignature for entities.
However, do you think it's a suitable way to compare Value Objects?
I think DomainSignature attribute is used to calculate the identity of the entity. I am surprised he didn't go as far as override the Equals methods.
Taken from DDD Blue Book: "An Entity is an object fundamentally defined not by its attributes, but by a thread of continuity and identity."
When you change the primordial attributes that define the identity of the entity, that's another entity. An entity isn't defined by the ID in the database (if you are saving it in a database after all). What you are saying is that he should have made them immutable. That a desirable thing, but not always possible.
Yes. For entities, equality, identity and hash should be only on Id. Nothing else.
If you want to use FirstName and LastName as identity, you should create custom value object and use that as Id.
Also, why is setter of Id protected? It should never be possible to change the Id once entity is created/persisted. So it should be private.
GetHashCode and Equals are overriden in the entity class: https://github.com/codai/Sharp-Lite/blob/master/SharpLiteSrc/app/SharpLite.Domain/Entity.cs
and use the Id if the entity is persisted.
So you could end up with the same entity persisted multiple times in the database?
@DaeMoohn if the entity is not persisted, then it uses the GetHashCode on ComparableObject, so you wouldn't get the entity persisted multiple times.
What is difference between Object Signature and Identity? Because those are not equal with ComparableObject and Entity hiearchy.
"Probably the worst point of this object model is that it is highly connected"
I have a similar problem in my domain: getting an Order you probably want to see also the Customer name; do you create two properties CustomerId and CustomerName in the Order entity or what else?
Not related to S#Sharp, but about the model, I would prefer the so called Query Object Pattern, in which the OR/M query receive a dignity of complexity and insulation. Always exposing IQueryable is not the key in order to me, but providing some objects that easily expose the essential information we need to query the model and obtain the result in the preferred shape.
Mauro, I think it's a good practice to separate your read operations from writes, especially if presentation needs pieces of data from different entitites. Maybe not in the strict CQRS way, but you could have DB Views including columns of related entities(e.g. Orders DB view containing CustomerId and CustomerName columns and so on)In this way you have only flat objects and automatic query generations from grids(telerik grid, jqGrid) is way easier.
In dealing with a scenario where you need, say, the Customer name and the corresponding Orders I thought the preferred approach was to use a "view" or "DTO" object that exposes just what you need?
I mainly work with WebForms, not MVC, so in my case I would have one Presenter/ViewModel per screen, so a screen that lists out orders by Customer would include just the specific customer information I need on that screen. I suppose in MVC you could do the same thing using a ViewModel.
Look at the new programming style! where? apple of course! Bret Victor http://vimeo.com/36579366
"It is the same as saying that by changing the shirt I wear, I become a completely different person."
I think a lot of fashion designers would agree with this statement :P
@YovannyR - I keep seeing people link to that video, but I checked out his blog and it's slow, unintuitive, and does not cooperate with a scrolling mouse. Not exactly ground breaking if you ask me.
"Probably the worst point of this object model is that it is highly connected, which encourages people to try to walk the object graphs where they should issue a separate query instead."
I couldn't agree more!
"Probably the worst point of this object model is that it is highly connected, which encourages people to try to walk the object graphs where they should issue a separate query instead."
I completely agree. I see this symptom very often and it is a sign of a misguided understanding of how to apply DDD. The notion of a read model, reporting object, or DTO doesn't seem to be very well expressed as a core pattern in applying DDD.
"Probably the worst point of this object model is that it is highly connected, which encourages people to try to walk the object graphs where they should issue a separate query instead."
Ayende, are you saying that the models should be segregated at the aggregate boundaries, e.g. Order should only reference CustomerId, not the Customer model? Doesn't that defeat the power of an ORM?
As a NHibernate user, I don't see "connecting" the models as a way or excuse to walk the object graph, its advantage is in ease of writing HQL, DetachedCriteria and linq queries. And all queries should be explicit about which associations or collections to eager load.
@Owen No. That means OrderLineItem shouldn't reference Order and Order shouldn't reference Customer. Eg. make the relation directional. This is explained in DDD book and leads to model, that is easier to understand.
I think thats what Ayende means.
@Falhar I'm pretty sure the ID has to have a protected (non-private) setter so that nHibernate can set it through reflection when loading an entity.
Comment preview