Entities: Required Properties And Properties That Shouldn’t Be Modified
Posted by Davy Brion on March 24th, 2009
How often do you see entities mapped with getters and setters for every property, and only a default constructor (either added implicitly by the compiler or explicitly by a developer)? It’s not really the best way to map entities, so i just wanted to show a better way of doing this.
Consider the OrderLine entity. It has 4 required properties: Order, Product, UnitPrice and Quantity. It also has one optional property called DiscountPercentage. The Order and Product properties should never be changed after the OrderLine was created. It also has a database Id property which should never be changed either.
This is how the code of the OrderLine class would look like:
public class OrderLine : IIdentifiable<int>
{
public OrderLine(Order order, Product product, decimal unitPrice, int quantity)
{
if (order == null) throw new ArgumentNullException("order");
if (product == null) throw new ArgumentNullException("product");
this.order = order;
this.product = product;
UnitPrice = unitPrice;
Quantity = quantity;
}
// required for NH
protected OrderLine() {}
private int id;
public virtual int Id
{
get { return id; }
}
private Order order;
public virtual Order Order
{
get { return order; }
}
private Product product;
public virtual Product Product
{
get { return product; }
}
public virtual decimal UnitPrice { get; set; }
public virtual int Quantity { get; set; }
public virtual double? DiscountPercentage { get; set; }
}
There is only one public constructor, which takes all of the required properties as parameters. The protected constructor is only there because NHibernate needs it to create run-time proxies (which enable all of the lazy-loading magic). In theory, you can’t create instances of the OrderLine entity without its required data.
Also, notice how the Id, Order and Product properties only have a getter, and no setter. These values can no longer be changed by developers once the object is constructed. The UnitPrice and Quantity properties do have setters, because these values can be modified after the entity is created.
The mapping for this class looks like this:
<class name="OrderLine" table="OrderLine" >
<id name="Id" column="Id" type="int" access="nosetter.camelcase" >
<generator class="identity" />
</id>
<many-to-one name="Order" column="OrderId" class="Order" not-null="true" access="nosetter.camelcase" />
<many-to-one name="Product" column="ProductId" class="Product" not-null="true" access="nosetter.camelcase" />
<property name="UnitPrice" column="UnitPrice" type="Decimal" not-null="true" />
<property name="Quantity" column="Quantity" type="int" not-null="true" />
<property name="DiscountPercentage" column="DiscountPercentage" type="double" />
</class>
It’s pretty easy… each property that shouldn’t be changed after creation is mapped with the nosetter.camelcase access strategy. That means NHibernate uses the private field to set the value directly after creation, but will use the getters whenever it needs to read the data from the entity.
As you can see, without too much trouble you can make sure that your entities always have their required data, and that properties that shouldn’t change after creation can’t be modified either.
March 24th, 2009 at 9:55 pm
Even better would be to have no public constructor and use a factory to create an OrderLine.
ie.
order.CreateOrderLine(product, quantity);
order.CreateOrderLine(product, IDiscount, quantity);
March 24th, 2009 at 10:10 pm
@Chris
interesting comment… first i thought “how could an Order create an OrderLine instance if its constructor was not public?”. Obviously, making the OrderLine constructor internal would enable the Order class to create instances of it, while at the same time allowing you to control the creation of OrderLines through its aggregate root (Order). Which is nice, assuming that the rest of your assembly doesn’t create OrderLine’s outside of the control of the aggregate root (which it shouldn’t, although it would be possible).
But for people who aren’t doing DDD, that might be pushing it a little bit too far.
March 24th, 2009 at 11:20 pm
Pushing it too far? Even outside of DDD, I still want entities valid.
March 25th, 2009 at 6:08 am
You could have used the private instance modifer for the Id, Order and Product properties.
public virtual int Id { get; private set; }
March 25th, 2009 at 9:31 am
[...] Entities: Required Properties And Properties That Shouldn’t Be Modified – Davy Brion looks at a common mistake that gets made with NHibernate mappings, having getter and setter properties for all persisted properties, and looks at how you can improve your entities by avoiding this [...]
March 25th, 2009 at 11:16 am
Davy, as you know it, it’s a good start but it quickly becomes more complex if you want to encapsulate more aspects of DDD while keeping your model consistent … and in the end, you definitely don’t want to do that manually again and again.
Would be nice if we could integrate a standardized way of creating a DDD model in ActiveWriter (too bad days are not 48 hours long lol).
Question : do you usually manage bi-directional relations in a generic way or you handle it in Create / Remove methods ?
March 25th, 2009 at 11:27 am
i use specific Add/Remove methods
as for having to keep it consistent manually… i still see the consequences of code generation taken too far on a semi-daily basis and i’m pretty much convinced by now that keeping your model consistent manually takes less long-term effort than any kind of code generation
March 25th, 2009 at 11:33 am
Depends on how far you want to scale … if you want multiple teams to adopt your best practices, you’d really wanna have a look at code generation ;o)
March 25th, 2009 at 11:37 am
trust me, i really don’t
March 25th, 2009 at 11:43 am
Sorry to insist ;o) lol
Why not integrate your knowledge in a tool so that others don’t have to go through the full learning curve you went through, and save yourself some lengthy code reviews ?
Isn’t it a possible solution to what you wrote here :
http://davybrion.com/blog/2009/03/why-dont-we-learn/
Integrate the collective knowledge in tools (patterns & best practices), so that we can continue to raise the abstraction levels … sounds good to me
)
March 25th, 2009 at 12:06 pm
if you merely generate code for others, without investing time into making sure that everyone knows _why_ the code looks like that, they usually won’t learn from it.
integrating patterns & best practices in tools is obviously good, but generated code doesn’t really fit into that category IMO, unless we’re talking about really small stuff. I’ve seen how code generation based on the database structure leads to a host of problems time and time again, and there’s really nothing that can get me to ever consider it again.
March 25th, 2009 at 1:59 pm
Doesn’t it make you cringe making all your properties virtual? Also, don’t you only have to make single-association end properties virtual and not simple types such as int, string etc, and not multi-associations (such as Order.Lines)?
March 25th, 2009 at 2:04 pm
@Peter
as far as i know, everything has to be virtual if you want NH to use proxies.
the fact that members aren’t virtual by default is what makes me cringe
March 25th, 2009 at 3:56 pm
[...] Entities: Required Properties And Properties That Shouldn’t Be Modified – Davy Brion [...]
March 25th, 2009 at 4:54 pm
This topic is one of my rant triggers so I apologize in advance.
I have this discussion often with devs I work with. “without too much trouble you can make sure that your entities always have their required data”. Who determines what data is required for an entity? The answer should be “the business”. Why enforce that at the entity level? Let the consumer of the entity assert whether or not the information is valid. Or delegate the validation to some business rules validator. Why don’t you take it all the way and make the quantity and unit price unsigned?
In your code I presume you never have a line that looks like this:
if( orderLine.Order != null ) or if( orderLine.Product != null )
Right? Because the Order and Product will never be null. Until some off the wall requirement is introduced.
I’m sure there are aspects of an OrderLine that can be tested without an order or a product.
March 25th, 2009 at 8:08 pm
@Bill
“Who determines what data is required for an entity?”
there is at least some minimum defined in your dabatase (if you use one), and those are the ones you might as well put in your constructor. No matter what the “business” says, as long as they are required in the database i’d prefer to put those in the constructor. If they really aren’t required anymore you can just lift the not-null restriction in the database and get rid of the argument in the constructor.
Until that off the wall requirement is introduced (_if_ it happens), i’d prefer protection against non-nullable database columns over a hypothetical flexibility that i might never need. And once that off the wall requirement comes along, i’d consider getting rid of the not-null constraint.
March 25th, 2009 at 8:28 pm
I completely agree with the gist of this post. I’ll propably go one step further: making a seperate method for changing the mutable as well. In this particular example you can get away with setters, but more involved examples require one atomic change of the mutable properties. This is way I generally consider a setter property a bad thing for entities. I wrote about this in the past:
http://elegantcode.com/2008/09/26/properties-a-false-sense-of-encapsulation/
March 27th, 2009 at 5:16 pm
There are 2 things at the moment really putting me off using NH.
1: Using proxies requires me to make my properties virtual, this makes me cringe! I only expose state when I need to, to make them virtual too makes me feel so dirty, and not the good kind of dirty.
2: I read (NHibernate in Action I believe) that when you have a multi association and the instances are really a mixture of base class instances and descendant instances the proxy approach will create a proxy of the base class for every entry. That CAN’T be right, can it?
March 28th, 2009 at 10:50 am
@Peter
no, if you have a one-to-many collection of say, Person objects where Persons can be either Customers or Employees or whatever, the collection you’ve used will not contain proxies of these instances… they will just be instances of the type they represent. The collection however, is the one that enables the lazy-loading in this case. Access the collection and it will retrieve the data if it’s not there yet, but the retrieved objects themselves will not be proxies (though their many-to-one properties will be proxy instances if lazy-loading isn’t disabled for those associations)