Ayende @ Rahien

Refunds available at head office

Challenge: The lazy loaded inheritance many to one association OR/M conundrum

Here is an interesting problem that pop up with any OR/M that supports inheritance and lazy loading.

Let us say that we have the following entity model:

image

Backed by the following data model:

image

As you can see, we map the Animal hierarchy to the Animals table, and we have a polymorphic association between Animal Lover and his/her animal. Where does the problem starts?

Well, let us say that we want to load the animal lover. We do that using the following SQL:

SELECT Name,
	 Animal,
	 Id
FROM AnimalLover
WHERE Id = 1 /* @p0 */

And now we have an animal lover instance:

var animalLover = GetAnimalLoverById(1);
var isDog = animalLover.Animal is Dog;
var isCat = animalLover.Animal is Cat;

Can you guess what would be the result of this code?

I’ll post the actual answer tomorrow…

Comments

marco
09/02/2009 07:30 AM by
marco

2 false becouse the type it's a proxy that inherits directly from Animal

Frans Bouma
09/02/2009 07:46 AM by
Frans Bouma

Can you guess what would be the result of this code?

depends on the O/R mapper you're using. If animalLover 1 has a dog, isDog is true for some O/R mappers, and otherwise it's false as Marco said.

Even though 'persistence ignorance' is thought to be used here, it's hardly 'ignorance', it's very tied to the underlying o/r mapper.

Tran Ngoc Van
09/02/2009 07:55 AM by
Tran Ngoc Van

I think both isDog and isCat are false.

Due to lazy loading, the Animal instance has to be a proxy, and thus cannot be Dog or Cat. At the time the Animal proxy is instantiated, we have not known its actual time.

Steve Powell
09/02/2009 08:24 AM by
Steve Powell

This has been the only real problem we have been having with nHibernate.

We have a parallel class hierarchy problem where we have Animal, Cat, Dog and AnimalHome, Kennel, CatBasket. Animal has a property LivesIn which is of type AnimalHome and Dog and Cat override this property with the 'new' keyword returning Kennel and CatBasket. If you try and go Cath.LivesIn you get a cast exception.

We have found a work around if anyone wants to know it..... but it aint pretty!

Steve Powell
09/02/2009 08:25 AM by
Steve Powell

oops. Should have been Cat.LivesIn not Cath.LivesIn. Not sure how the wife managed to creep into my comment there....

Anders Ivner
09/02/2009 09:02 AM by
Anders Ivner

This is one of the few (only?) design choices that nHibernate does wrong, imho. Alternately you could:

  1. take the penalty of joining the associated table to determine the actual type of Animal when loading the AnimalLover.

  2. Let the AnimalLover lazy load the animal when accessing the Animal property.

Alas, nhibernate sacrifices correctness for performance.

Ayende Rahien
09/02/2009 10:27 AM by
Ayende Rahien

Anders,

Liskov would be rolling in her grave, if she had one

Anders Ivner
09/02/2009 10:54 AM by
Anders Ivner

Not all valid usage scenarios are classic OO. You may want to use the object for e.g. serialization or data binding using reflection (similar to how NH itself works).

John Chapman
09/02/2009 12:29 PM by
John Chapman

Ok,

Let's make this more interesting. What if the base Animal had a method called ComeHere(). A cat ignores you because cats don't listen, and the dog comes. Calling that method instead of checking the type, would you get the correct polymorphic behavior? Assuming that this does behave correctly, you can work around this issue as long as you are aware of it.

While using NHibernate I never used lazy loaded properties to try to avoid situations like these. It was perfectly acceptable in our application.

Ayende Rahien
09/02/2009 12:33 PM by
Ayende Rahien

John,

You will get the right behavior, absolutely!

Schrödinger
09/02/2009 12:39 PM by
Schrödinger

no, the cat is dead when you check for it... so it must be dog

Ayende Rahien
09/02/2009 12:46 PM by
Ayende Rahien

Schrödinger,

That is aweseom

Roger Alsing
09/02/2009 01:18 PM by
Roger Alsing

Anders,

Liskov would be rolling in her grave, if she had one

What does his statements have to do with Liskov substitution?

NPersist did load the related entities on property access (when using lazy load that is)

And thus, we always got an object of the correct type.

So your statement that this is try for all mappers is simply not correct.

You eiter lazy load using ghosts, or lazy load on property or field access.

(And I assume that POCO or not POCO and how to accomplish POCO is out of the scope of this post)

Ayende Rahien
09/02/2009 01:23 PM by
Ayende Rahien

Roger,

Loading on property access means that you kill a bunch of very important optimizations:

animalLover.Animal.Id <-- no load

session.Save(new TreatmentRecord{

Animal = animalLover.Animal <-- no load

});

Roger Alsing
09/02/2009 01:39 PM by
Roger Alsing

That doesnt make the initial statement true.

animalLover.Animal.Id <-- no load

You could accomplish this using correct proxies also depending on how the mapping is created.

If the FK from animal lover to animal contains the type descriminator then you could very well instance a ghost proxy of the correct type.

Also, assigning objects of the wrong type to an association might be an optimization but it will also enable a bunch of problems.

And IMO, those problems are more related to Liskov.

Let's say that you'r setters of the .Animal property contains logic that makes decisions based on the type of the assigned object.

e.g.

set{

if (value is dog)

SendEmailIfDogLacksPuppyVaccination()

this.animal = value;

}

One can ofcourse argue about how valid that code is, but it does prove that a mapper that uses ghost proxies would break rule of least suprise and that it would potentially break pre/post conditions of your domain logic .

All of the above is somewhat offtopic, I'm just trying to say that your initial statement is not true and thus you shouldnt emphasise "any"

Roger Alsing
09/02/2009 01:57 PM by
Roger Alsing

hehe, I'll bite even more on this.

This introduces what we called "Leaky this" problem.

The actual animal instance and the ghost proxy that is assigned to the Animal property will not be the same instance.

As you explained, the ghost proxy will delegate any call to the real instance.

So if the Animal class would contain a method called "SetMeAsFavoriteAnimal(AnimalLover lover)"

e.g.

void SetMeAsFavoriteAnimal(AnimalLover lover)

{

lover.Animal = this;

}

then consider the following code:

AnimalLover lover1 = LoverRepo.GetLoverById(1);

AnimalLover lover2 = new AnimalLover();

lover1.Animal.SetMeAsFavoriteAnimal(lover2);

if (lover1.Animal == lover2.Animal)

//THIS WILL NOT EXECUTE!!!

Because lover1.Animal is a ghost proxy and lover2.Animal is a real animal instance.

And this kind of problems are a PAIN to find.

I do understand you have chosen this approach from pragmatic reasons, but I think it is conceptually wrong.

kork
09/02/2009 02:04 PM by
kork

roger hibernate works with the concept of entity and 2 entity can be compared only by their id, so in your example you must compare using

lover1.Animal.Id == lover2.Animal.Id or redefine Equals and the == operator to works using only the Id.

conceptually the choise it's the better choise available

Ayende Rahien
09/02/2009 02:25 PM by
Ayende Rahien

Roger,

Nope, you can't do that without hitting the actual table container the data. That affect the general case vs. the special case.

And you code still violate Liskov

Ayende Rahien
09/02/2009 02:26 PM by
Ayende Rahien

Roger,

Yep, leaky this is a real and annoying issue.

We run around this a few times trying to resolve that, but there isn't a good solution that doesn't kill the general case for the special case.

However, NH says that entities SHOULD implement Equals, GetHashCode & Equality operators, because of that exact reason.

Tyler Burd
09/02/2009 03:21 PM by
Tyler Burd

This is why I stopped using inheritance with NH a year ago. I tried using the interface proxies to get around the issues, but it never felt right having to create interfaces for my persistent classes and remember to use them only within an inheritance hierarchy. I switched to simple many-to-ones, where Animal would have Dog and Cat properties, and I've never been happier! The team understands what's going on, they can change the type of animal from cat to dog, and they don't get any of these "is it or isn't it a dog?!" issues.

I think the inheritance is very well done in NH, I just don't like having to consider so many special cases.

Mr_Simple
09/02/2009 03:43 PM by
Mr_Simple

@Tyler

I switched to simple many-to-ones, where Animal would have Dog and Cat properties, and I've never been happier!

Mr_Simple approves of your coding style!

gunteman
09/02/2009 06:07 PM by
gunteman

"And you code still violate Liskov "

Why? How?

Ayende Rahien
09/02/2009 06:09 PM by
Ayende Rahien

See Replace conditional with polymorphism.

gunteman
09/02/2009 06:43 PM by
gunteman

Oh,you mean the "is" check? I agree about that one, but I fail to see how Anders' (not Richard's) statement has anything to do with that.

NHibernate has made a conscious decision in this matter. It serves a very real and valuable purpose. Doesn't make it "the right" decision though.

Fabio Maulo
09/02/2009 07:34 PM by
Fabio Maulo

Guessing that a Cat can't be a lover of a Dog I would imagine a different implementation.

ivos
09/02/2009 08:56 PM by
ivos

hey, if you have an animal, you don't have a cat or a dog. you have an animal. you shouldn't make any decision based on the type. Instead, you should use a polymorphic solution (like the one John said, a Visitor, etc). And what about overriding methods with "new"?!!? I think all this problems will be solved once the people that uses ORMs understands the O.

Frank
09/02/2009 09:44 PM by
Frank

How much of a real problem is it?

Unfortunately at my current company, we engage in anemic models, and to make it even more problematic, data sets with a custom written data layer for each system. And open source libraries like NHibernate are a no-go, because too many people think they need to be in full control of everything and open source is not to be trusted.

But back to the question. In my very limited experience with models, I don't see why one would check what the type of the property is. The reason why one uses inheritance is to provide different behavior. Checking for a type looks like behavior at the wrong place. John Chapman seems to be spot on with his example.

Roger Alsing
09/03/2009 07:03 AM by
Roger Alsing

@Frank

Its way to easy to say "use polymorphism instead of conditionals and every problem will go away"

It does solve some problems, yes..

But what if you pass the object to some custom serializer?

What if you place it in a hashtable?

Also, never forget that people are people and will most likely do the most quick n dirty solution they can come up with.

And don't get me wrong, Im not trying to bash NHibernate or anything.

I'm just saying that O/R mapping is hard enough any way for most people.

So adding extra friction in terms of object comparison and identity problems will not make it easier for anyone.

Alex Yakunin
09/03/2009 11:21 AM by
Alex Yakunin

Just read this post (after reading & commenting a subsequent one about the same). I'm fully agree, such a practice is quite bad.

Moreover, this is completely contradictorily to Liskov principal (in fact, you're talking about inverse principal at all!): this principal does not imply you must expose absolutely the same logic as in base type. It implies any subclass instance can replace superclass instance everywhere. But you're doing COMPLETELY INVERSE THING!

Do you understand that such imagination of Liskov substitution principal makes polymorphism impossible at all?

Alex Yakunin
09/03/2009 11:32 AM by
Alex Yakunin

And... I also surprised to hear this: "However, NH says that entities SHOULD implement Equals, GetHashCode & Equality operators, because of that exact reason.".

I'm curious, what is the typical implementation? I.e. 2 things are clear for me:

  • Such instances are frequently used in sets & dictionaries

  • Thus their hash code shouldn't change

  • Likely, two instances with the same key must be equal

Thus I see the only good implementation of these methods: they must rely only on key comparison & key hash code.

If there will be anything else, you'll get mutable GetHashCode = forget about dictionaries.

Mats Helander
09/04/2009 04:34 PM by
Mats Helander

I switched to simple many-to-ones, where Animal would have Dog and Cat properties, and I've never been happier!

"Mr_Simple approves of your coding style!"

Same here.

Alex Yakunin
09/05/2009 06:02 AM by
Alex Yakunin

I switched to simple many-to-ones, where Animal would have Dog and Cat properties, and I've never been happier!

In some cases this is possible (and likely, desirable), although in general such a design won't work. I.e. it is a solution working in particular cases.

Comments have been closed on this topic.