ChallengeThe 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:
Backed by the following data model:
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…
More posts in "Challenge" series:
- (01 Jul 2024) Efficient snapshotable state
- (13 Oct 2023) Fastest node selection metastable error state–answer
- (12 Oct 2023) Fastest node selection metastable error state
- (19 Sep 2023) Spot the bug
- (04 Jan 2023) what does this code print?
- (14 Dec 2022) What does this code print?
- (01 Jul 2022) Find the stack smash bug… – answer
- (30 Jun 2022) Find the stack smash bug…
- (03 Jun 2022) Spot the data corruption
- (06 May 2022) Spot the optimization–solution
- (05 May 2022) Spot the optimization
- (06 Apr 2022) Why is this code broken?
- (16 Dec 2021) Find the slow down–answer
- (15 Dec 2021) Find the slow down
- (03 Nov 2021) The code review bug that gives me nightmares–The fix
- (02 Nov 2021) The code review bug that gives me nightmares–the issue
- (01 Nov 2021) The code review bug that gives me nightmares
- (16 Jun 2021) Detecting livelihood in a distributed cluster
- (21 Apr 2020) Generate matching shard id–answer
- (20 Apr 2020) Generate matching shard id
- (02 Jan 2020) Spot the bug in the stream
- (28 Sep 2018) The loop that leaks–Answer
- (27 Sep 2018) The loop that leaks
- (03 Apr 2018) The invisible concurrency bug–Answer
- (02 Apr 2018) The invisible concurrency bug
- (31 Jan 2018) Find the bug in the fix–answer
- (30 Jan 2018) Find the bug in the fix
- (19 Jan 2017) What does this code do?
- (26 Jul 2016) The race condition in the TCP stack, answer
- (25 Jul 2016) The race condition in the TCP stack
- (28 Apr 2015) What is the meaning of this change?
- (26 Sep 2013) Spot the bug
- (27 May 2013) The problem of locking down tasks…
- (17 Oct 2011) Minimum number of round trips
- (23 Aug 2011) Recent Comments with Future Posts
- (02 Aug 2011) Modifying execution approaches
- (29 Apr 2011) Stop the leaks
- (23 Dec 2010) This code should never hit production
- (17 Dec 2010) Your own ThreadLocal
- (03 Dec 2010) Querying relative information with RavenDB
- (29 Jun 2010) Find the bug
- (23 Jun 2010) Dynamically dynamic
- (28 Apr 2010) What killed the application?
- (19 Mar 2010) What does this code do?
- (04 Mar 2010) Robust enumeration over external code
- (16 Feb 2010) Premature optimization, and all of that…
- (12 Feb 2010) Efficient querying
- (10 Feb 2010) Find the resource leak
- (21 Oct 2009) Can you spot the bug?
- (18 Oct 2009) Why is this wrong?
- (17 Oct 2009) Write the check in comment
- (15 Sep 2009) NH Prof Exporting Reports
- (02 Sep 2009) The lazy loaded inheritance many to one association OR/M conundrum
- (01 Sep 2009) Why isn’t select broken?
- (06 Aug 2009) Find the bug fixes
- (26 May 2009) Find the bug
- (14 May 2009) multi threaded test failure
- (11 May 2009) The regex that doesn’t match
- (24 Mar 2009) probability based selection
- (13 Mar 2009) C# Rewriting
- (18 Feb 2009) write a self extracting program
- (04 Sep 2008) Don't stop with the first DSL abstraction
- (02 Aug 2008) What is the problem?
- (28 Jul 2008) What does this code do?
- (26 Jul 2008) Find the bug fix
- (05 Jul 2008) Find the deadlock
- (03 Jul 2008) Find the bug
- (02 Jul 2008) What is wrong with this code
- (05 Jun 2008) why did the tests fail?
- (27 May 2008) Striving for better syntax
- (13 Apr 2008) calling generics without the generic type
- (12 Apr 2008) The directory tree
- (24 Mar 2008) Find the version
- (21 Jan 2008) Strongly typing weakly typed code
- (28 Jun 2007) Windsor Null Object Dependency Facility
Comments
2 false becouse the type it's a proxy that inherits directly from Animal
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.
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.
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!
oops. Should have been Cat.LivesIn not Cath.LivesIn. Not sure how the wife managed to creep into my comment there....
This is one of the few (only?) design choices that nHibernate does wrong, imho. Alternately you could:
take the penalty of joining the associated table to determine the actual type of Animal when loading the AnimalLover.
Let the AnimalLover lazy load the animal when accessing the Animal property.
Alas, nhibernate sacrifices correctness for performance.
Anders,
Liskov would be rolling in her grave, if she had one
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).
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.
John,
You will get the right behavior, absolutely!
no, the cat is dead when you check for it... so it must be dog
Schrödinger,
That is aweseom
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)
oops.. try = true
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
});
That doesnt make the initial statement true.
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"
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)
{
}
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.
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
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
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.
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.
@Tyler
Mr_Simple approves of your coding style!
"And you code still violate Liskov "
Why? How?
See Replace conditional with polymorphism.
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.
Guessing that a Cat can't be a lover of a Dog I would imagine a different implementation.
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.
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.
@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.
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?
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.
"Mr_Simple approves of your coding style!"
Same here.
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.
Comment preview