Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to override equals without increasing cyclomatic complexity?

I was recently overriding some equals methods in domain objects of my recent Java project. As we are using Sonar to calculate our code metrics, I immediately saw the cyclomatic complexity of these classes increasing above a threshold.

I'm wondering if there is a clever way, pattern or option at all to keep this metric low although having a little more complex equals method.

EDIT: Here is one of my examples that I have, nothing really specific I would say, just so that we know what we are talking about.

@Override
public boolean equals(Object o) {
  if (o instanceof MyKey) {
  MyKey other = (MyKey) o;
  if (this.foo.longValue() == other.getFoo().longValue() &&
    this.bar.equalsIgnoreCase(other.getBar()) &&
    this.foobar.shortValue() == other.getFoobar().longValue()){
    return true;
    }
  }
  return false;
}

@Override
public int hashCode() {
  int hash = 3;
  hash = 53 * hash + foo.hashCode();
  hash = 53 * hash + bar.hashCode();
  hash = 53 * hash + foobar.hashCode();
  return hash;
}
like image 487
hecko84 Avatar asked Jan 26 '17 12:01

hecko84


2 Answers

You could use Apache's EqualsBuilder:

public boolean equals(Object obj) {
  if (obj == null) { return false; }
  if (obj == this) { return true; }
  if (obj.getClass() != getClass()) {
    return false;
  }
  MyClass rhs = (MyClass) obj;
  return new EqualsBuilder()
             .appendSuper(super.equals(obj))
             .append(field1, rhs.field1)
             .append(field2, rhs.field2)
             .append(field3, rhs.field3)
             .isEquals();
}
like image 112
john16384 Avatar answered Sep 29 '22 13:09

john16384


You didn't but you should always check for nulls. foo could be null, resulting in a NullPointerException.

this.foo.longValue() == other.foo.longValue()

Luckily Objects utility class saves you from a lot of problems as it automatically checks for nulls.

@Override
public boolean equals(Object object) {
    if (object == null)
        return false;

    if (!(object instanceof MyObject))
        return false;

    MyObject other = (MyObject) object;

    //@formatter:off
    return Objects.equals(getX(), other.getX()) &&
           Objects.equals(getY(), other.getY()) &&
           Objects.equals(getZ(), other.getZ()));
    //@formatter:on
}

@Override
public int hashCode() {
    return Objects.hashCode(getX(), getY(), getZ());
}

If the fields to check are a lot you can optionally add this at the beginning of the equals method.

if (object == this)
    return true;

In theory it can save some computation in some edge case.

The only thing that really helps, in my opinion, is good indentation. I always wrap those line between a pair of //@formatter:off and //@formatter:on. It's boilerplate code, anyway: very easy to write, very easy to make mistakes.

In your case, though, you're checking equality using equalsIgnoreCase. It's a pity Objects doesn't have such a method. You can build your own pretty easily.

public final class Strings {
    public static boolean equalsIgnoreCase(String a, String b) {
        return a == null ? b == null : a.equalsIgnoreCase(b);
    }

    private Strings() {
    }
}

And use it like this

return Objects.equals           (getX(), other.getX()) &&
       Strings.equalsIgnoreCase (getY(), other.getY()) &&
       Objects.equals           (getZ(), other.getZ()));
like image 38
mneri Avatar answered Sep 29 '22 13:09

mneri