Uploaded image for project: 'Rules Repository'
  1. Rules Repository
  2. RSPEC-2162

"equals" methods should be symmetric and work for subclasses

    Details

    • Type: Bug Detection
    • Status: Active
    • Resolution: Unresolved
    • Labels:
    • Message:
      Hide
      * Compare to "this.getClass()" instead.
      * Remove this comparison to an unrelated class.
      Show
      * Compare to "this.getClass()" instead. * Remove this comparison to an unrelated class.
    • Default Severity:
      Minor
    • Impact:
      Low
    • Likelihood:
      Low
    • Covered Languages:
      Java
    • Remediation Function:
      Constant/Issue
    • Constant Cost:
      5min
    • Analysis Scope:
      Main Sources, Test Sources
    • CERT:
      MET08-J.
    • FindBugs:
      EQ_OVERRIDING_EQUALS_NOT_SYMMETRIC, EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS, EQ_GETCLASS_AND_CLASS_CONSTANT

      Description

      A key facet of the equals contract is that if a.equals(b) then b.equals(a), i.e. that the relationship is symmetric.

      Using instanceof breaks the contract when there are subclasses, because while the child is an instanceof the parent, the parent is not an instanceof the child. For instance, assume that Raspberry extends Fruit and adds some fields (requiring a new implementation of equals):

      Fruit fruit = new Fruit();
      Raspberry raspberry = new Raspberry();
      
      if (raspberry instanceof Fruit) { ... } // true
      if (fruit instanceof Raspberry) { ... } // false
      

      If similar instanceof checks were used in the classes' equals methods, the symmetry principle would be broken:

      raspberry.equals(fruit); // false
      fruit.equals(raspberry); //true
      

      Additionally, non final classes shouldn't use a hardcoded class name in the equals method because doing so breaks the method for subclasses. Instead, make the comparison dynamic.

      Further, comparing to an unrelated class type breaks the contract for that unrelated type, because while thisClass.equals(unrelatedClass) can return true, unrelatedClass.equals(thisClass) will not.

      Noncompliant Code Example

      public class Fruit extends Food {
        private Season ripe;
      
        public boolean equals(Object obj) {
          if (obj == this) {
            return true;
          }
          if (obj == null) {
            return false;
          }
          if (Fruit.class == obj.getClass()) { // Noncompliant; broken for child classes
            return ripe.equals(((Fruit)obj).getRipe());
          }
          if (obj instanceof Fruit ) {  // Noncompliant; broken for child classes
            return ripe.equals(((Fruit)obj).getRipe());
          }
          else if (obj instanceof Season) { // Noncompliant; symmetry broken for Season class
            // ...
          }
          //...
      

      Compliant Solution

      public class Fruit extends Food {
        private Season ripe;
      
        public boolean equals(Object obj) {
          if (obj == this) {
            return true;
          }
          if (obj == null) {
            return false;
          }
          if (this.getClass() == obj.getClass()) {
            return ripe.equals(((Fruit)obj).getRipe());
          }
          return false;
      }
      

      See

      • CERT, MET08-J. - Preserve the equality contract when overriding the equals() method

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                ann.campbell.2 Ann Campbell
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: