The general consensus seems to be that using the primary key in
equals()/hashCode() if it exists and falling back to the Object
implementations is the best solution except for the issue of hashCode()
changing when the object is persisted and the primary key is set.
An answer to this issue is to keep the original return value of
hashCode() *and* make sure if the same entity is loaded in another
session that it's hashCode() returns the same value as well.
Here's an solution that implements this:
public class Entity
{
private static final Map<Integer, Integer> SAVED_HASHES =
Collections.synchronizedMap(
// Use a WeakHashMap so entries will be
// garbage collected once all entities
// referring to a saved hash are
// garbage collected themselves
new WeakHashMap<Integer, Integer>() );
private Integer id;
private volatile Integer hashCode;
public Integer getId()
{
return id;
}
protected synchronized void setId( Integer id )
{
// If we've just been persisted and hashCode has been
// returned then make sure other entities with this
// ID return the already returned hash code
if ( (this.id == null) &&
(id != null) &&
(hashCode != null) ) {
SAVED_HASHES.put( id, hashCode );
}
this.id = id;
}
public boolean equals( Object obj )
{
if ( this == obj ) {
return true;
} else {
return (getId() != null) &&
(obj instanceof Entity) &&
getId().equals( ((Entity) obj).getId() );
}
}
public int hashCode()
{
if ( hashCode == null ) {
synchronized ( this ) {
if ( hashCode == null ) {
Integer newHashCode = null;
if ( id != null ) {
newHashCode = SAVED_HASHES.get( id );
}
if ( newHashCode == null ) {
if ( id != null ) {
newHashCode = id;
} else {
newHashCode = super.hashCode();
}
}
hashCode = newHashCode;
}
}
}
return hashCode;
}
}
The only impact of this change is the synchronization and WeakHashMap
constantly checking for stale entries it can expunge. But I don't think
this is too much of a problem given we're dealing with a database as
well. The Map could probably just be a ConcurrentHashMap because it's
only a map of Integers to Integers and therefore won't consume much
memory but a WeakHashMap should mean memory and performance will remain
fairly constant and therefore have no scaling issues.
Can anyone see anything wrong with this solution?
P.S. This is actually a cut down version of what I've developed which is
a generic super class that handles this for all my entity classes. |