Must Everything Be Virtual With NHibernate, Part III
Posted by Davy Brion on September 27th, 2009
In the previous post i showed a piece of code which suffers from 2 problems and asked you guys to spot both problems. One of the problems was pointed out in the comments, but nobody mentioned the other one.
Once again, this is the code example:
Instead of making everything virtual, only properties that are eligible for lazy-loading have been made virtual. Now, the line of code that is problematic is obviously the one where the DiscountPercentage of the customer backing field is accessed. I will address using the field instead of the property later on in this post so bear with me for now.
There are 2 situations in which this code will fail badly. The first situation is when you’re dealing with a proxy of an Order. If you have an uninitialized proxy of Order, and you call the non-virtual CreateOrderLine method then you will get a NullReferenceException because the proxy can’t intercept the call to CreateOrderLine, and because it also can’t intercept accessing the customer field, which will be null at that time. This problem was correctly spotted by one of the people who left a comment.
The other problem is far worse, IMO. Suppose you have a genuine instance of an Order object, but its reference properties haven’t been loaded yet and are uninitialized proxies. If there is no requirement for every public member of a proxy-able type to be virtual, then we can pretty much assume that the DiscountPercentage of the Customer class is also non-virtual. Which means that with this code, we have no possible way of intercepting the call to the Customer’s proxy’s DiscountPercentage property. Unfortunately, DiscountPercentage will have its default value of 0, so you won’t get an exception… instead, the customer gets no discount even though its record in the database might have a discount set.
The possibility of running into one of these problems due to usage of an ORM and depending on how an object is loaded is simply unacceptable. Some people commented on the usage of the customer backing field instead of the Customer property with a “then simply don’t do that” response. That’s not an acceptable solution to this problem, it’s a lame workaround at best. There are plenty of reasons why people would use the backing field in their code instead of the property and if they run into this problem, they definitely will blame the ORM. And rightfully so, i might add.
September 28th, 2009 at 7:00 pm
Lame workaround, hmmm…
I would still argue that the example is a little contrived to support the point. If I was reviewing this code I would say that the Customer property should be an auto-property like the rest.
I would argue that, in this code, referring to the backing field is a bug/smell/bad practice.
It has always just struck me as odd that I need to open up the API to my classes to satisfy the ORM.
I also find it hard to agree with your last point about blaming the ORM. I would suggest that these case were backing fields are accessed are smells. I would be looking to see where my code wasn’t conforming to the correct conventions not blaming the ORM for not being clever enough.
I’m playing devils advocate here. I am willing to be convinced.
September 28th, 2009 at 8:14 pm
What if i don’t want to publicly expose certain values outside of my entity? You could argue that you could do that with protected automatic properties, but what if i don’t want those values to be available to derived classes either?
Regardless of what your personal preferences are, people will use backing fields for various reasons in their entities. The possibility of not initializing lazy-loaded values because of that, is something that an ORM should prevent.
“It has always just struck me as odd that I need to open up the API to my classes to satisfy the ORM.”
i find it just as odd that i would need to open up the data of my classes to _everyone_ because of a limitation in an ORM
September 29th, 2009 at 4:01 pm
OK. Better example.
But no reason you couldn’t just use a private property though ?
The reason I keep coming back to this point is that we’ve never needed this scenario. So making everything virtual has never seemed justifiable.
One of the reasons to switch to NHibernate is because of the very smart things it can do. But it’s difficult to to warrant touching all the persisted classes to make everything virtual in order to support a scenario we don’t (and probably won’t) ever use.
Two questions if I may:
1. Without studying the proxy generator. How do I gain some confidence that it “does the right thing” in _all_ circumstances?
2. Presumably there’s no reason why I can’t fork my own proxy generator that conforms to my property only scheme. Should be _much_ simpler right?
September 29th, 2009 at 6:45 pm
how could a dynamic proxy intercept a private property? you can’t influence them through inheritance so the proxies can’t do anything with them
1) i haven’t looked into the non-castle proxy generators, but the one for castle only requires that every non-private member is virtual and that you have a default constructor (it doesn’t even need to be public)
2) you can actually use your own proxy factory and generator… there is out-of-the-box support for Castle, LinFu and Spring. If you want your own proxy generator, you just need to provide an implementation… though i’m not entirely sure if all of the checks have been removed from the NHibernate codebase and have been moved into the specific proxy factories (though i guess they have been)
October 2nd, 2009 at 7:55 am
Sorry, but this discussion is largely pointless, and your code sample is not what I would call best practice.
Just use interfaces together with the
proxykeyword in the mapping, and don’t care about virtual or not virtual or some weird under-the-hood behaviour of dynamic proxies – it is a bad smell that the domain logic has to know about these issues at all!And it’s in my experience also much better to let NH populate fields directly (via
nosetter.camelcase) instead of using the relating properties, because the public properties of a business class will often have some additional validation orPropertyChangedlogic, which I don’t want to be executed when only hydrating an entity from the DB. Besides all other things, this can lead to massive performance problems…As you said, you also can hook your own IoC-Container into the proxy creation process, removing even the need for a default c’tor. But I don’t know yet if this is desirable in every case, since this may create other performance issues (IoC containers are very flexible, but also very slow…)
- Thomas
October 2nd, 2009 at 8:43 am
@Thomas
it really all depends on what you prefer… the whole point is that you can do pretty much everything that you prefer, with some very minor (IMO) constraints
October 6th, 2009 at 6:37 am
How can I code this example with NHibernate
public class SomeObject
{
//Hibernate mapped field
protected virtual EncryptedDatabaseField { get; set; }
internal ClearTextField
{
get { return DecryptField(this.EncryptedDatabaseField); }
set { this.EncryptedDatabaseField = EncryptField(value); }
}
}
I simply trying to prevent see data ouside my assembly. If I will make ClearTextField “protected internal virtual” then it expose access to encryped database fields to anyone who can just simply subclass my class.
October 6th, 2009 at 8:24 am
@Pavel
you could just use the field access strategy
October 6th, 2009 at 3:29 pm
Thank you for replay. We use NHibernate for some time already. I got into trouble after trying upgrade to 2.1 release. My DAL objects has a few methods that marked as internal virtual. Idea behind it that I want restrict execution to my assemblies only for security reason. On top of it we obfuscate all internal marked stuff to add some extra trouble for people who wants to debug what we do. Now hibernate requires me to change all my
internal virtual SomeMethod(); into protected internal virtual and that I am trying to avoid doing this. I can make NHibernate to be “friend” of my assembly and in this case you should have no problem override my internal virtual stuff. Is it going to work?
October 7th, 2009 at 6:53 am
I’m not sure, but i don’t think nhibernate keeps track of InternalsVisibleTo so i doubt that it will work
October 7th, 2009 at 6:49 pm
Thank you for answer. We will set Lazy = false on all of our objects. As understand it will prevent NHibernate from generatig proxy on our objects. Correct if I am wrong, collections of child objects will be still lazy loaded. The difference will be that this collection will contains instances of object itself not proxies. I it right?
October 8th, 2009 at 8:31 am
hmm, i’m not entirely sure that the collections will still be lazy loaded if you set lazy=false