5

I'm working on a project where many classes need proper typical implementations of equals and hashCode: each class has a set of final fields initialized at construction with "deeply" immutable objects (nulls are intended to be accepted in some cases) to be used for hashing and comparison.

To reduce the amount of boilerplate code, I thought about writing an abstract class providing common implementations of such behavior.

public abstract class AbstractHashable {

    /** List of fields used for comparison. */
    private final Object[] fields;

    /** Precomputed hash. */
    private final int hash;

    /**
     * Constructor to be invoked by subclasses.
     * @param fields list of fields used for comparison between objects of this
     * class, they must be in constant number for each class
     */
    protected AbstractHashable(Object... fields) {
        this.fields = fields;
        hash = 31 * getClass().hashCode() + Objects.hash(fields);
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == this) {
            return true;
        }
        if (obj == null || !getClass().equals(obj.getClass())) {
            return false;
        }
        AbstractHashable other = (AbstractHashable) obj;
        if (fields.length != other.fields.length) {
            throw new UnsupportedOperationException(
                    "objects of same class must have the same number of fields");
        }
        for (int i=0; i<fields.length; i++) {
            if (!fields[i].equals(other.fields[i])) {
                return false;
            }
        }
        return true;
    }

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

}

This is intended to be used like this:

public class SomeObject extends AbstractHashable {

    // both Foo and Bar have no mutable state
    private final Foo foo;
    private final Bar bar;

    public SomeObject(Foo foo, Bar bar) {
        super(foo, bar);
        this.foo = Objects.requireNonNull(foo);
        this.bar = bar; // null supported
    }

    // other methods, no equals or hashCode needed

}

This is basically what proposed here with some differences.

This seems to me a straightforward yet good approach to reduce verbosity and still have efficient implementations of equals and hashCode. However, as I don't recall to have ever seen something similar (except in the answer linked above), I would like to specifically ask whether is there some point against this approach (or possibly some improvement which could be applied), before applying it across the whole project.

Community
  • 1
  • 1
rrobby86
  • 1,356
  • 9
  • 14
  • 1
    I see two issues already. 1. Two objects will be considered equal only if *all* their fields match. This may not always be what you want. 2. A class can only extend one other class. Do you really want to lock your classes from inheriting other classes because `equals` and `hashCode` reuse is more important? – Chetan Kinger Jan 25 '17 at 09:32

4 Answers4

5

I see two issues with this approach already :

  1. Two objects will be considered equal if and only if all their fields match. This is not always the case in the real world.
  2. By making your classes extend from AbstractHashable, you can no longer extend from any other classes. This is quite a high price to be paying for reusing equals and hashCode.

The first issue could be solved by passing more meta data to your AbstractHashable class that allows it to identify what fields are optional. For instance, you could pass another array to AbstractHashTable that contains index positions to be ignored as its elements via a setter. The second issue can be solved by using Composition. Instead of extending AbstractHashTable, refactor it so that it can establish a HAS-A relationship with its users rather than an IS-A relationship.

However, as I don't recall to have ever seen something similar (except in the answer linked above), I would like to specifically ask whether is there some point against this approach

This approach will definitely impact the readability aspect of your code. If you can come up with a more readable approach (say by using annotations), I don't see anything wrong with wanting to reuse equals and hashCode implementations.

That all being said, modern IDEs such as eclipse can easily generate the equals and hashCode implementation for you so is there really a need to come up with a solution of this sort? I believe no.

Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82
  • 1
    Thanks, these are two interesting points (especially the 1st, which I didn't think about). Autogeneration by Eclipse is what I was using when I came up with the question: I didn't like all that boilerplate code even if it is autogenerated (also because changes like adding a field become more error-prone), but I'll probably deal with it for now. – rrobby86 Jan 26 '17 at 13:38
0

Based on my experience this seems bad as you will increase the error you can get in runtime vs. compile time (remember all use of these objects in lists etc. can now give you unexpexted behaviour). What if the order of fields in the classes are different? Secondly are you misusing inheritance? Maybee opt for some framework (https://projectlombok.org/) which generate hashcode and equals based on annotation?

HoleInVoid
  • 39
  • 6
  • *What if the order of fields in the classes are different*. This will never be an issue as the constructor in the subclasses (`SomeObject` for example) will always enforce a fixed order in which the fields will be intialized. Besides, you are comparing objects of the same class so how will the field order change? – Chetan Kinger Jan 25 '17 at 09:43
  • You are right, you are comparing objects of same class, but then why do you have if (fields.length != other.fields.length)? – HoleInVoid Jan 25 '17 at 10:07
  • the OP has added that check for extreme cases where developers don't use the same constructor to instantiate the object. – Chetan Kinger Jan 25 '17 at 10:09
  • But the fields will still be there? – HoleInVoid Jan 25 '17 at 10:14
0

The solution should work.

However it feels a bit weak from OOP point of view:

  • you are duplicating data (fields in superclass vs field in object), so your object is twice as large; also if you ever need to make them mutable, you'd need to maintain state in two places
  • the class hierarchy is forced, only to provide equals/hashCode

Modern IDEs generate such methods quite easily, if you want you can also use frameworks (like Lombok) or libraries (like Guava's Objects/Java 8's Objects) to simplify these methods.

Adam Kotwasinski
  • 4,377
  • 3
  • 17
  • 40
0

I would suggest to have a look at apache's HashCodeBuilder and EqualsBuilder classes. It has reflection based methods like reflectionHashCode and reflectionEquals as well.

  • Thank you for your suggestion. However, reflection has not optimal performances and I think it could be noticeable for an object having a large graph of dependent objects (which could happen in my case). – rrobby86 Jan 26 '17 at 13:30