What is wrong with my equals method?

Question

I am writing a Complex number class and my equals method doesn't seem to be working. I'm supposed to be checking to see if two ComplexNum objects are equal to each other and I am always coming across a logic error that always returns false.

public boolean equals(Object x) {
    ComplexNum real = (ComplexNum) x;
    if (x == null)
        return false;
    if (x instanceof ComplexNum && this.imag == (real.real)) {
        return true;
    } else
        return false;
}

This is my demo class

ComplexNum y = new ComplexNum(3.0,15.0);
ComplexNum z = new ComplexNum(3.0,15.0);

    System.out.println(y.equals(z));

false < -- my output is false no matter what the values are

Look up testing for floating point equality. That's your problem, since the digital representation of a floating point number is *never* exact (except in certain infrequent circumstances) and so `==` should not be used to test for equality of these entities. — Hovercraft Full Of Eels, Oct 04 '17 at 21:49
You are comparing the imaginary part of **this** to the **real** part of the other. Also, don't name the other one "real", that's very confusing. — user949300, Oct 04 '17 at 22:00
I don't think you should be comparing `real` of one value to `imag` of the other, should you? — Fred Larson, Oct 04 '17 at 22:00
IMHO, this is not the place for arguments about whether epsilon comparisons are more appropriate. IMHO (again) the `.equals()` method should perform _exactly_ what `==` does for floats, and epsilon-based comparison should be done in another method. — Alnitak, Oct 04 '17 at 23:03
NB: this original code would cause a `ClassCastException` if the passed object is not a `ComplexNum` because you perform the cast in the first line, without checking the type with `instanceof` first. — Alnitak, Oct 06 '17 at 12:06

Alnitak · Accepted Answer · 2017-10-06T12:03:59.843

You're not performing a like-for-like comparison on the real and imaginary components of your complex number.

This would be my implementation:

public boolean equals(Object o) {
    if (o instanceof ComplexNum) {
        ComplexNum other = (ComplexNum)o;
        return (this.real == other.real) && (this.imag == other.imag);
    } else {
        return false;
    }
}

For those arguing that floating point comparison should never use precise tests, IMHO that should be in another method. Java requires that objects that are .equal() have the same .hashCode(), and that would be very hard to arrange with an epsilon comparison.

Because of the above, you will need this, too (other implementations are possible, and the below assumes that your components are double)

int hashCode() {
    return Double(this.real).hashCode() ^ Double(this.imag).hashCode();
}

NB: A test for o == null is not necessary in .equals(), because null is not an instanceof ComplexNum.

EDIT for completely strict conformance with the way that .equals() works on Float or Double types (particularly for NaN values) you might want to use this line instead which replaces == with a per-component application of .equals()

return Double(this.real).equals(other.real) &&
       Double(this.imag).equals(other.imag);

What is wrong with my equals method?

1 Answers1