16f6c0aecf
Inline adoc files when they are included exactly once. Also fix language tags because this inlining gives us better information on what language the code is written in.
116 lines
3.7 KiB
Plaintext
116 lines
3.7 KiB
Plaintext
== 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[]
|