-->
These old forums are deprecated now and set to read-only. We are waiting for you on our new forums!
More modern, Discourse-based and with GitHub/Google/Twitter authentication built-in.

All times are UTC - 5 hours [ DST ]



Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 15 posts ] 
Author Message
 Post subject: Bag.equals() ?
PostPosted: Tue Sep 30, 2003 4:17 am 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
Hi,
Is there a reason why n.s.h.collection.Bag does not implements the equals and hashcode methods (as j.u.List implementations does in jdk) ?

Emmanuel


Top
 Profile  
 
 Post subject:
PostPosted: Tue Sep 30, 2003 4:22 am 
Hibernate Team
Hibernate Team

Joined: Tue Aug 26, 2003 12:50 pm
Posts: 5130
Location: Melbourne, Australia
A bag is not really a list, hence we don't need to implement the whole contract.


Top
 Profile  
 
 Post subject:
PostPosted: Tue Sep 30, 2003 5:34 am 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
Ok but is it inapropriate to compare 2 bags and saying that having a proper implementation of bag.equals and bag.hashcode ?

My underlying question is "Can I implement an equals and hashcode method, with respect to the bag semantic w/o breaking hibernate way of working ?"

I'm ready to do it.


Top
 Profile  
 
 Post subject:
PostPosted: Tue Sep 30, 2003 5:36 am 
Hibernate Team
Hibernate Team

Joined: Tue Aug 26, 2003 12:50 pm
Posts: 5130
Location: Melbourne, Australia
I would rather that bag had no equals()/hashCode(), since I don't like the idea that equals() forces the collection to be initialized.


Top
 Profile  
 
 Post subject:
PostPosted: Tue Sep 30, 2003 9:37 am 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
But you did it on the set implementation, probably for the same purpose, no ?

Code:
equals() {
    read();
    ...
}


I don't wan't to disturb you ;-), I just think using bag equals right now can be very confusing, and try to convince you or being convinced by you.

Emmanuel


Top
 Profile  
 
 Post subject:
PostPosted: Tue Sep 30, 2003 9:46 am 
Hibernate Team
Hibernate Team

Joined: Tue Aug 26, 2003 12:50 pm
Posts: 5130
Location: Melbourne, Australia
In the case of a Set, we need it to satisfy the Set contract.


Top
 Profile  
 
 Post subject:
PostPosted: Wed Oct 01, 2003 3:04 am 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
OK Gavin, take a big icy coke and answer this post ;-)
Bag.equals method isn't used that much (assuming the current one is buggy - at least have a strange behavior).
Don't you think a programmer calling myCollection.equals(anotherOne) understand that he needs to load it and thus assumes that perf ussue ?


Top
 Profile  
 
 Post subject:
PostPosted: Wed Oct 01, 2003 10:54 am 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
OK I did the patch for the 2.0.3 version (can't have access to head right now)

http://opensource.atlassian.com/projects/hibernate/secure/ViewIssue.jspa?key=HB-375

I don't think bag.equals is widely used, so the read() call won't be a issue in common usage, and Object.equals semantic is now more appropriate.

Hope you guys will accept it.

Emmanuel


Top
 Profile  
 
 Post subject:
PostPosted: Wed Oct 01, 2003 11:17 am 
Hibernate Team
Hibernate Team

Joined: Tue Aug 26, 2003 12:50 pm
Posts: 5130
Location: Melbourne, Australia
Well, I'm not absolutely certain.

I always thought it was a really crap to have persistent collections getting initialized when they were used in other collections, etc.

I -could- be convinced to change my mind on this, but other people would also need to request it. (I don't make changes that break existing code lightly.)


Top
 Profile  
 
 Post subject:
PostPosted: Fri Oct 03, 2003 2:55 am 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
Actually, I agree with you. Blind usage of collection comparison is bad thing in persistent fwk, and I advise not to do it (the 'understand what yo're doing' advice). But the proper answer of the underlying framework is to:
- avoid it by throwing an exception during equals call
- implementing it properly and document it as a bad usage.


Top
 Profile  
 
 Post subject: This is a bug.
PostPosted: Sat Jul 17, 2004 6:58 pm 
Newbie

Joined: Sat Jul 17, 2004 6:34 pm
Posts: 2
Really, it is. My collegue just spent the better part of an afternoon figuring out why removal of an object from a list was failing and we found that Bag indeed does not implement equals, and, unfortunately, nowhere in Javadocs is it documented that Bag partially implements the Collections framework contracts.

Apart from the fact that documentation does not make this clear, the issue is that Bag is silently failing its contract, which is never good behavior. I agree with Emmanuel; this should either be overriden to throw an exception (in which case Bag would still be failing its contract but at least making some noise about it so hapless newbies like ourselves can work around quickly) or implemented with a note of caution.

The bottom line is, this omission breaks required behavior for the sake of an optimization, with no way of overriding it and no documentation to warn inexperienced users.

BTW, the performance reasoning you give only applies to lazily loaded collections. If the collection is not lazy loading (the *default* behavior) there is no additional overhead.


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 17, 2004 11:07 pm 
Hibernate Team
Hibernate Team

Joined: Mon Aug 25, 2003 9:11 pm
Posts: 4592
Location: Switzerland
Quote:
BTW, the performance reasoning you give only applies to lazily loaded collections. If the collection is not lazy loading (the *default* behavior) there is no additional overhead


Note that this is legacy and we advise you to use lazy loading for collections by default whenever possible.

_________________
JAVA PERSISTENCE WITH HIBERNATE
http://jpwh.org
Get the book, training, and consulting for your Hibernate team.


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 17, 2004 11:41 pm 
Hibernate Team
Hibernate Team

Joined: Tue Aug 26, 2003 12:50 pm
Posts: 5130
Location: Melbourne, Australia
If it is intended functionality, it is, by definition, not a bug.


Top
 Profile  
 
 Post subject:
PostPosted: Sun Jul 18, 2004 1:04 pm 
Newbie

Joined: Sat Jul 17, 2004 6:34 pm
Posts: 2
I think it is a stretch to call not implementing Bag.equals() intended functionality; it has to be actually useful before we can do that, not to mention being documented. It seems to me that comparing two fully initialized collections in your entity model should always just work, you don't expect the meaning of collection equality to be broken when you have fully fetched all necessary data. A sensible implementation of Bag.equals() can throw an exception when either collection is not initialized (or return false to keep the current behavior) but it should work correctly and as expected otherwise.


Top
 Profile  
 
 Post subject:
PostPosted: Wed Jul 21, 2004 7:48 am 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
Just Apply my patch, it should still work even on Hibernate 2.1.
I'll add a comment on the Javadoc to make it more clear.

_________________
Emmanuel


Top
 Profile  
 
Display posts from previous:  Sort by  
Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 15 posts ] 

All times are UTC - 5 hours [ DST ]


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
© Copyright 2014, Red Hat Inc. All rights reserved. JBoss and Hibernate are registered trademarks and servicemarks of Red Hat, Inc.