rspec/rules/S2162/java/rule.adoc

== Why is this an issue?

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

[source,java]
----
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

[source,java]
----
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;
}
----


== Resources

* https://wiki.sei.cmu.edu/confluence/x/AzZGBQ[CERT, MET08-J.] - Preserve the equality contract when overriding the equals() method


ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

* Compare to "this.getClass()" instead.
* Remove this comparison to an unrelated class.


'''
== Comments And Links
(visible only on this page)

=== replaces: S2161

=== on 15 Oct 2014, 21:50:59 Freddy Mallet wrote:
We're advising the opposite in the compliant solution of rule RSPEC-2097 @Ann. And we're doing that because 95% of the time, developers are using ``++instanceof++`` operator which is more readable than checking the equality of classes. 


I guess this rule might remain valuable in some specific contexts but I would not activate it by default. 

=== on 16 Oct 2014, 15:44:56 Ann Campbell wrote:
\[~freddy.mallet] I've updated RSPEC-2097 because the old example was broken for child classes since the advice is to use the parent class' ``++equals++`` method for parent class fields and check child class fields in the child's ``++equals++`` method.

=== on 16 Oct 2014, 16:36:20 Ann Campbell wrote:
FYI [~freddy.mallet], I followed, then reversed your advice and combined this with EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS instead since both rules look at symmetry.

=== on 17 Oct 2014, 10:07:04 Freddy Mallet wrote:
Ok @Ann and I would merge this rule with RSPEC-2161

endif::env-github,rspecator-view[]