Through the eyes of sonar : equals & hashCode.

Equals and hashCode

Theory

equals

Indicates whether some other object is “equal to” this one.

The equals method implements an equivalence relation on non-null object references:

– It is reflexive: for any non-null reference value x, x.equals(x) should return true.
— It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
— It is transitive: for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.
— It is consistent: for any non-null reference values x and y, multiple invocations of x.equals(y) consistently return true or consistently return false, provided no information used in equals comparisons on the objects is modified.
— For any non-null reference value x, x.equals(null) should return false.

The equals method for class Object implements the most discriminating possible equivalence relation on objects; that is, for any non-null reference values x and y, this method returns true if and only if x and y refer to the same object (x == y has the value true).

Note that it is generally necessary to override the hashCode method whenever this method is overridden, so as to maintain the general contract for the hashCode method, which states that equal objects must have equal hash codes.

hashCode

Returns a hash code value for the object. This method is supported for the benefit of hashtables such as those provided by java.util.HashMap or java.util.HashSet.

The general contract of hashCode is:

– Whenever it is invoked on the same object more than once during an execution of a Java application, the hashCode method must consistently return the same integer, provided no information used in equals comparisons on the object is modified. This integer need not remain consistent from one execution of an application to another execution of the same application.
— If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.
— It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables.

As much as is reasonably practical, the hashCode method defined by class Object does return distinct integers for distinct objects. (This is typically implemented by converting the internal address of the object into an integer, but this implementation technique is not required by the JavaTM programming language.)

Recommandations

Use the commons lang classes to help your code to be “nullsafe” and easier to implement :

EqualsBuilder
HashCodeBuilder

If you are using hibernate :
— use the getter to avoid “proxy issues”
— avoid using the id as equality (which isn’t always filled)
— read the faq

If you implement them… unit test your equals/hashCode
— There’s a EqualsHashCodeTestCase that you can extends in the junit-addons library
— note that there are also constraint for serialisation/clone
— don’t call hashCode() but toHashCode() on hashCodeBuilder ;)
— avoid reflection based equals/hashCode.

Unit testing

the EqualsHashCodeTestCase is extended in order to test a class’s functional compliance with the equals and hashCode contract.
Override my createInstance and createNotEqualInstance methods to provide me with objects to test against. Both methods should return objects that are of the same class.
will test :
— whether equals holds up against a new Object (should always be false).
— whether equals holds up against null.
— whether equals holds up against objects that should not compare equal.
— whether equals is consistent.
— whether equals is reflexive.
— whether equals is symmetric and transitive.
— the hashCode contract.
— the consistency of hashCode Across Invocations .

Unit Tests and Serialization and serialization

If you adopt a unit-testing methodology, then any serializable class should pass the following three tests:
— If it implements readObject(), it should implement writeObject(), and vice-versa.
— It is equal (using the equals() method) to a serialized copy of itself.
— It has the same hashcode as a serialized copy of itself.

Similar constraints hold for classes that implement the Externalizable interface.

Sonar rules

Warn – eclipse generated equals check class

	public boolean equals(Object obj) {
		if (this == obj)
			return true;
		if (obj == null)
			return false;
		if (getClass() != obj.getClass()) // BAD IDEA IF USING HIBERNATE
			return false;

Prefer instanceof check, because the “obj” may be a HibernateProxy dynamically extending MyObject

	public boolean equals(Object obj) {
		if (this == obj){
			return true;
                }
		if (obj == null) {
			return false;
                }
		if (!(obj instanceof MyObject)) {
			return false;
		}

For the fields comparisons… prefer the EqualsBuilder

see http://tomaszdziurko.pl/2009/05/eclipse-and-problem-with-auto-generated-equals/

Bad practice – Class defines equals() and uses Object.hashCode()

This rule checks that classes that override equals() also need to override hashCode(). If you implement equals… you should implement hashCode accordingly.

Bad practice – equals() method does not check for null argument

As stated in the theory… “whether equals holds up against null”.
A simple check with instanceof… could do the trick.

 public boolean equals(final Object object) {
   if (object.getClass().equals(this.getClass())) {
 	DecisionDetail decisionDetail = (DecisionDetail) object;
    	if (getId() == null || decisionDetail.getId() == null) {
    		return object == this;	
   	}
   }
 }

See openpojo to test your model against such issue.

Correctness – equals method always returns false / Correctness – equals method always returns true

Having an equals method like this isn’t a good idea and doesn’t follow the contract on these methods

//BAD
 public boolean equals(Object object) {		
   return true; // or false			
 }

If you want a good default implementation

//GOOD
 public boolean equals(Object object) {		
   return this==object;
 }

Performance – hashCode return a constant

Returning a constant for the hashCode isn’t good for performance, as the hash is use to “compartiment/bucketing” data in structure like HashSet/hashMap.
Returning a constant will put all the element in the same bucket transforming a lookup of O(1) to O(n) function.

If you don’t have a clean equals criteria fallback to default implementation :

  @Override
  public int hashCode() {
 	return System.identityHashCode(this);
  }
  public boolean equals(Object object) {		
   return this==object;
  }

Correctness – equals() used to compare array and nonarray

Comparing apples and oranges, an array of X won’t be equals to an object instance X

  !PersonTypeCode.ALL.equals(searchParameter.getPersonTypeCode())

Where

  public class PersonTypeCode extends Code {
      public static final PersonTypeCode[] ALL = { PROSPECT, MEMBER, PROSPECT_OR_MEMBER, PROSPECT_FAMILY_MEMBER };
      public static final PersonTypeCode PROSPECT = new PersonTypeCode("PROSPECT");
      ...
   }

Equals vs == :Literal Equality

Literal Strings should be compared using equals(), not ‘!=’.

 String text = ...
 if (text != "") { //BAD
   ...
 }

Ideally… you should use StringUtils to compare in a null safeway.
Note that you have StringUtils.isEmpty(...) or StringUtils.isBlank(...)

 String text = ...
 if (!StringUtils.isEmmpty(text)) {
    ...
 }

Class defines equal(Object); should it be equals(Object)?

Surely a typo. If it’s not a typo choose a better name.

Class defines hashcode(); should it be hashCode()?

Surely a typo. If it’s not a typo choose a better name.

Class defines tostring(); should it be toString()?

Surely a typo. If it’s not a typo choose a better name.

Dodgy – Potentially dangerous use of non-short-circuit logic

 return equalsRoot(other) & new EqualsBuilder().append(this.sourceId, other.getSourceId()).isEquals();

This code seems to be using non-short-circuit logic (e.g., & or |) rather than short-circuit logic (&& or ||). In addition, it seem possible that, depending on the value of the left hand side, you might not want to evaluate the right hand side (because it would have side effects (hibernate initialization), could cause an exception (NullPointerException) or could be expensive (hibernate initialization).

 String s = null;   
 if (s == null || s.trim().length() == 0) {  
     // String s is really empty. Do some stuff.  
 }      
 if (s != null && s.trim().length() != 0) {  
    // String s is not empty. Do some stuff.  
 } 

Both examples would throw a NullPointerException when s is null and you use | and & respectively. (Prefer StringUtils for this kind of check)

Non-short-circuit logic causes both sides of the expression to be evaluated even when the result can be inferred from knowing the left-hand side. This can be less efficient and can result in errors if the left-hand side guards cases when evaluating the right-hand side can generate an error.

About these ads

, , ,

  1. Leave a comment

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Follow

Get every new post delivered to your Inbox.

%d bloggers like this: