CODEWITHSUNDEEP
java
nimo23
nimo23

Reputation: 5680

equals and compare with BigDecimal

I have a class overriding hashCode() and equals()-method. As I deal with BigDecimal, I have to use compareTo() instead of Objects.equals():

public class MyProduct extends Product{

private BigDecimal price;


@Override
public int hashCode() {
    return Objects.hash(price);
}


@Override
public boolean equals(Object obj) {

        if (this == obj) return true; // is this right or should this be deleted

        if (obj == null || getClass() != obj.getClass()) return false;

        final Product other = (Product) obj;

        // is this right?
        if (price == null ? (other.price != null) : price.compareTo(other.price) != 0) {
            return false;
        }
        return super.equals(obj);
    }

}

I have the following questions:

  1. Should I delete the line if (this == obj) return true; from equals()-method? Because with this line, the compareTo is not triggered and a wrong equals() could be computed, am I right?
  2. Could the equals()-method be improved?

Upvotes: 1

Views: 1605

Answers (3)

nimo23
nimo23

Reputation: 5680

I found the simplest way:

public static boolean isEqual(BigDecimal val1, BigDecimal val2) {
    return val1 != null ^ val2 != null && val2 != null && val1.compareTo(val2) != 0;
}

and then use it within equals():

public boolean equals(Object obj) {
        if (this == obj) return true;
        if (obj == null || getClass() != obj.getClass()) return false;
        final MyProduct other = (MyProduct) obj;

        if(!isEqual(price, other.price)) return false;

        return super.equals(obj);
    }

Upvotes: 0

nimo23
nimo23

Reputation: 5680

I wrote an utitly method which can be used to compare two BigDecimals without throwing NPE:

// returns true, if val1 is the same as val2
// can this be improved ?
public static boolean isEqual(BigDecimal val1, BigDecimal val2) {
        return !((val1 != null ^ val2 != null) || (val2 != null && val1.compareTo(val2) != 0));
    }

This can be used within equals-method:

@Override
    public boolean equals(Object obj) {
        if (this == obj) return true;
        if (obj == null || getClass() != obj.getClass()) return false;
        final MyProduct other = (MyProduct) obj;

        if(!isEqual(price, other.price)) return false;

        return super.equals(obj);
    }

Upvotes: 0

Mateusz Herych
Mateusz Herych

Reputation: 1605

The first line is just an optimization meant to early return a result if both references point to the same object.

Is price nullable? I assume it is, as you're checking for it within your equals() implementation. In that case your code won't work in case other.price will be null. Specifically this code here:

price.compareTo(other.price) != 0

Will throw a NullPointerException.

You can fix it like this:

    @Override
    public boolean equals(Object obj) {

        if (this == obj) return true; // is this right or should this be deleted

        if (obj == null || getClass() != obj.getClass()) return false;

        final MyProduct other = (MyProduct) obj;

        // If you prefer, the below two can be replaced with a single condition
        // price != null ^ other.price != null
        // Credits to @Andreas
        if (price == null && other.price != null) {
            return false;
        }
        if (price != null && other.price == null) {
            return false;
        }
        if (other.price != null && price.compareTo(other.price) != 0) {
            return false;
        }

        return super.equals(obj);
    }

Now, you could probably make it shorter, but personally I find it the most readable this way.

Anyways, unless you really, really care about customising your equals() implementation, I would suggest generating one with your IDE and sticking to it. They do a decent job most of the time and you don't have to worry about it being broken (although comparing BigDecimals may be tricky for them, given you don't care about the scale but rather just the value).

Upvotes: 1

Related Questions