Hi Richard,
Any luck since the last post?
Here's a few things to work with:
- Remove the comparator from your set mapping. You can put it back in later, but get rid of it for now. It's only going to make things muddier, and it may be the source of some of your problems.
- Rewrite hashCode from scratch. There are a few things that your modified hashCode still does that it must not do:
Code:
public int hashCode () {
if (Integer.MIN_VALUE == this.hashCode)
This is intending to cache the hashCode, meaning that any any "fixes" you do to an object between the time it has hashCode first calculated and the time it is fully configured and ready for saving are ignored. For example, if you have a Person class whose business key is their name and social security number, and the constructor you use needs only a name parameter, you could do something create a Person, print out its hashCode (instantly calculating and caching its hashCode), set its security number (which should change its hashCode, but doesn't), then add it to a set. It's going to go into the wrong hashbucket. This code also insulates you from a very common bug: adding an item to a set before its business key is fully known. Because the hashCode won't change while you continue to set up its hashCode, bugs that should be fairly easy to detect will go unnoticed.. until some maintenance developer spots the poor hashCode code and fixes it... and then spends another few days finding all the bugs that were previously hidden.
Code:
{
if (null == this.getId()) {
return super.hashCode();
}
This will return different hashCodes depending on whether or not the object has been saved. This is very bad, and is exactly why hibernate-persisted objects should use only their business keys to determine their hashCode.
Code:
else {
String hashStr = this.getCampWeek().getId().toString() + ":" + this.getChild().getId().toString();
this.hashCode = hashStr.hashCode();
}
Nothing wrong with the algorithm, but this is a very inefficient way of calculating a hashCode from two ints.. create four temporary strings (week.id.toString(), week.id.toString() + ":", child.id.toString(), and finally hashStr) then get a hashCode from one of those strings. Why not just go with the tried and true "37 + 17xVal1 + 17xVal2 + 17xVal3..."? Or use apache-commons' HashCodeBuilder class.
Code:
}
System.out.println("Id is: " + this.getId() + ", hashCode is: " + this.hashCode);
return this.hashCode;
}
This is the actual caching of the hashCode. See the earlier comment.
This hashCode has the right intentions, but suffers from a more subtle issue (that the above hashCode also sufferts from):
Code:
public int hashCode()
{
if ((getChild() == null) || (getCampWeek() == null))
{
throw new IllegalArgumentException("ChildInCampWeek object not fully set up, cannot compute hashCode yet");
}
return 37 + 17 * getChild().getId() + 17 * getCampWeek().getId();
}
The problem with this one is that hibernate does not guarantee that child objects are loaded in any particular order. Even though the code works perfectly when the objects are user-created, when they're hibernate-created this code will cause IllegalArgumentExceptions (if the objects are immediately put into hibernate-mapped collections). Business keys must be properties of an object, not properties of an object's nested objects.
You need to do one of two things to get this hashCode to work (and hopefully, to solve all of your problems at the same time). Either add a new property to ChildInCampWeek to use as its business key, or store the Child id and the CampWeek id directly in ChildInCampWeek, and use those ids as its business key. I'd favour the second solution: even though it's more complex to map, I think that it's more accurate. In your hibernate mapping, add the two ids to the ChildInCampWeek mapping, and use formula="ChildId"/formula="CampWeekId" (and not column="..."), as this will allow you to retain the existing many-to-one mappings. There's some java overhead, because you need to ensure that the methods that hibernate uses to populate the various attributes aren't the same as the methods available to the API user: you need an internal and an external version of setChild() and setCampWeek(), and an internal-only version of setChildId() and setCampWeekId(). Then you can use getChildId() and getCampWeekId() to work out the hashCode.
Note that the equals method doesn't have to worry about any of this, as it's not called when adding CampInCampWeek objects to hibernate-mapped sets, so it will have access to getChild() and getCampWeek(). hashCode() cannot guarantee that those methods will return sensible values, and thus cannot use them.
Whew. Now to really throw the spanner in the works: a fairly simple redesign of your model should make this problem go away. You could get rid of the ChildInCampWeek object, and instead put a set of Childs directly into CampWeek (it would be a set containing a many-to-many, rather than a one-to-many: your DB schema probably won't change). Then the most dificult hashCode would just disappear.