10

I implement equals() the Java 7 way:

@Override
public boolean equals(Object obj)
{
    if (this == obj) return true;
    if (obj == null) return false;
    if (getClass() != obj.getClass()) return false;
    MyClass other = (MyClass) obj;
    return Objects.equal(myFirstField, other.myFirstField) &&
           Objects.equal(mySecondField, other.mySecondField);
}

Is there a way to reduce the code duplication?

I would prefer something like

@Override
public boolean equals(Object obj)
{
    if (Objects.equalsEarlyExit(this, obj)) return Objects.equalstEarlyExitResult(this, obj);
    MyClass other = (MyClass) obj;
    return Objects.equal(myFirstField, other.myFirstField) &&
           Objects.equal(mySecondField, other.mySecondField);
}

Or similar.

Roger C S Wernersson
  • 6,362
  • 6
  • 34
  • 45
  • The Objects class is Java 7 standard. I want it to do more. – Roger C S Wernersson Aug 07 '14 at 13:36
  • 1
    @WilliamF.Jameson Basically he wants a shorthand for the first 3 lines of code which are always the same (boilerplate code). – Fabian Barney Aug 07 '14 at 13:39
  • You can't re-open classes in Java. You can subclass your *own* objects that implement common behavior, but that won't apply to objects outside of your control. You could also play devious bytecode games to get part of the way there, but if you're using an IDE, it requires IDE support, a la Lombok. – Dave Newton Aug 07 '14 at 13:39
  • @FabianBarney It is clear after the edit, but `differ` did not sum that up. – William F. Jameson Aug 07 '14 at 13:39
  • 3
    @RogerWernersson Why don't you just write these helper methods (which would encapsulate the boilerplate code) yourself? I mean, if a method is not there, it's not there. – peter.petrov Aug 07 '14 at 13:41
  • Sidenote: When you override equals, always override hashCode, too! – Fildor Aug 07 '14 at 13:48
  • 1
    @peter.petrov It is a legitimate (and indeed advisable) approach to ask if one is missing something which may already have been provided by the JDK. – William F. Jameson Aug 07 '14 at 13:49
  • 1
    Does the removal of 3 lines of code warrant the addition of 2 method calls? – John B Aug 07 '14 at 13:50
  • 1
    @JohnB This is not just about line count; it is about respecting the best practice with as little chance as possible of going wrong about it. – William F. Jameson Aug 07 '14 at 13:51
  • Why not let the IDE of your choice to generate equals(), hashCode() and toString() for you? – Nikola Kolev Aug 07 '14 at 13:52
  • @WilliamF.Jameson Did I say it's not legitimate? I was just giving an idea. – peter.petrov Aug 07 '14 at 13:52
  • 1
    @peter.petrov In your comment you already assume OP *knows* a method is not there, or anywhere else in the JDK, and that the JDK also does not sport any other API with the same effect. That's assuming too much. – William F. Jameson Aug 07 '14 at 13:54
  • 1
    @WilliamF.Jameson I'm now assuming you assume what I assume. Never mind. OK :) – peter.petrov Aug 07 '14 at 13:55
  • 2
    @NikolaKolev I for one never let that happen because it creates a whole screenful of boilerplate code. Signal-to-noise ratio plummets. – William F. Jameson Aug 07 '14 at 13:58

4 Answers4

4

Standard API Java with autoboxing and object creation inefficiencies:

import static java.util.Arrays.*;
import java.util.List;

class BrevityBeforeEfficiency {
  int foo;
  Object bar;
  boolean baz;

  @Override
  public boolean equals(Object obj) {
    return (obj instanceof BrevityBeforeEfficiency)
        && ((BrevityBeforeEfficiency) obj).values().equals(values());
  }

  @Override
  public int hashCode() {
    return values().hashCode();
  }

  private List<?> values() {
    return asList(foo, bar, baz);
  }
}
McDowell
  • 107,573
  • 31
  • 204
  • 267
  • Interesting. I have heard that the use of insanceof is very inefficient, so I'm reluctant use it. I like the use of values() which allows for really short methods, which we could implement in a common base class. Although i really should be part of Object to be really nice. I guess we could add rules to Sonar or similar that no class, except the new base class, may inherit directly from Object. – Roger C S Wernersson Aug 10 '14 at 10:19
  • instanceof won't incur significant costs in a modern JVM - whether to use instanceof or getClass() is a design decision you need to make depending on the inheritance strategy/class contract. More of a concern for the above approach are the heap allocation/garbage collection costs. Every invocation of `values()` causes the allocation of an array for the varargs and the instantiation of a `List` type to decorate it. The autoboxing of the `long` value may also cause a new `Long` to be created. – McDowell Aug 10 '14 at 17:15
  • Also, if you plan to move to Java 8 there are [things you can do with lambdas](http://illegalargumentexception.blogspot.co.uk/2014/08/java-easy-equalshashcodetostring-less.html). – McDowell Aug 11 '14 at 17:54
  • Won't I get a NullPointerException if obj is null? – Roger C S Wernersson Jan 09 '15 at 07:54
  • `obj instanceof BrevityBeforeEfficiency` guards against null. null is not an instance of any type. – McDowell Jan 09 '15 at 10:04
2

You can use org.apache.commons.lang.builder.EqualsBuilder from commons-lang

Example:

public boolean equals(Object other) {
    return org.apache.commons.lang.builder.EqualsBuilder.reflectionEquals(this, other);
}

Other example:

private boolean equalsHelper(Object obj) {
    if (obj == null) return false;
    if (getClass() != obj.getClass()) return false;
    return true;
}


public boolean equals(Object obj) {

    if (this == obj) return true;

    if(!equalsHelper(ob)) {
      return false;
    }

    MyClass other = (MyClass) obj;
    return new EqualsBuilder()
      .append(myFirstField, other.myFirstField)
      .append(mySecondField, other.mySecondField).isEquals()
}
slavik
  • 1,223
  • 15
  • 17
  • This has a completely different use case than what OP is asking about. It is quite recognizable from the question that OP is looking for a solution which performs *equally well* as the long `equals` form. – William F. Jameson Aug 07 '14 at 13:50
2

mixing in a bit of inheritance:

public abstract class BusinessObject
{
    protected abstract Object[] getBusinessKeys();

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

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

        if(obj == this) return true;

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

        BusinessObject other = (BusinessObject) obj;

        return Arrays.deepEquals(this.getBusinessKeys(), other.getBusinessKeys());
    }
}

so the only boilerplate code is to extend BusinessObject and the single-lined getBusinessKeys():

public class Node extends BusinessObject
{
    private final String code;
    private final String name;

    public Node(String code, String name)
    {
        this.code = code;
        this.name = name;
    }

    @Override
    protected Object[] getBusinessKeys()
    {
        return new Object[] { code, name };
    }
}

It's the simplest and cleanest that I can think :)

Michele Mariotti
  • 7,372
  • 5
  • 41
  • 73
0

Here could be an implementation:

public abstract class EqualsHelper<T> {

    @SuppressWarnings("unchecked")
    public static <U> boolean equals(U that, Object other, EqualsHelper<U> equalsHelper) {
        return that == other || other != null && that.getClass().equals(other.getClass()) && equalsHelper.equals(that, (U) other);
    }

    public abstract boolean equals(T that, T other);

}

Then:

@Override
public boolean equals(Object obj) {
    return EqualsHelper.equals(this, obj, new EqualsHelper<MyClass>() {

        @Override
        public boolean equals(MyClass that, MyClass other) {
            return Objects.equal(that.myFirstField, other.myFirstField)
                && Objects.equal(that.mySecondField, other.mySecondField);
        }

    });
}

I wonder if this can be considered as an anti-pattern, so don't hesitate to blame me if you think it actually is ;)

sp00m
  • 47,968
  • 31
  • 142
  • 252