2

As its a pain to handle structural changes of the class in two places I often do:

class A {
  class C{}
  class B{}
  private B bChild;
  private C cChild;

  private Object[] structure() {
    return new Object[]{bChild, cChild};
  }

  public int hashCode() {
      Arrays.hashCode(structure());
  }

  public boolean equals(Object that) {
    //type check here
    return Arrays.equals(this.structure(), ((A)that).structure());
  }
}

What's bad about this approach besides boxing of primitives? Can it be improved?

Basilevs
  • 22,440
  • 15
  • 57
  • 102
  • Apart from the fact that it doesn't compile, you mean? So far, it has nothing to recommend it whatsoever. – user207421 Oct 24 '13 at 03:55
  • What does this approach try to solve? – Vitaliy Oct 24 '13 at 11:58
  • @Vitaliy, to encapsulate object structure in two places instead of three (fields, equals, hashCode). – Basilevs Oct 24 '13 at 13:54
  • And what JDK are you using? (is it 1.7?) – Vitaliy Oct 24 '13 at 14:20
  • I'm using 1.6 and 1.7. Thinking about migration to 1.7. – Basilevs Oct 24 '13 at 14:22
  • @Basilevs, in that case you might want to look at the java.util.Objects class in JDK 7. It actually implements a hash and equals utility in a manner that reminds what you wrote. The point being that this approach is actually sanctioned by JDK developers. Ernest Friedman-Hill has a point but in the majority of cases I don't think that the extra few machine instructions are worth saving at the expense of readability. – Vitaliy Oct 24 '13 at 20:54

3 Answers3

0

It's a clever way to reuse library methods, which is generally a good idea; but it does a great deal of excess allocation and array manipulation, which might be terribly inefficient in such frequently used methods. All in all, I'd say its cute, but it wouldn't pass a review.

Ernest Friedman-Hill
  • 80,601
  • 10
  • 150
  • 186
0

In JDK 7 they added the java.util.Objects class. It actually implements a hash and equals utility in a manner that reminds what you wrote. The point being that this approach is actually sanctioned by JDK developers. Ernest Friedman-Hill has a point but in the majority of cases I don't think that the extra few machine instructions are worth saving at the expense of readability.

For example: the hash utility method is implemented as: 

public static int hash(Object... values) {
    return Arrays.hashCode(values);
}
Vitaliy
  • 8,044
  • 7
  • 38
  • 66
-2

Someone familiarizing themselves with the code will have a bit more difficulty seeing what's going on. It's less "obvious" than listing the individual fields, as demonstrated by my previously erroneous answer. It is true, that "equals" is generally implemented with an "Object" passed in, so it's debatable, but the input is cast after the reference equality check. That is not the case here.

One improvement might be to store the array as a private data member rather than create it with the structure method, sacrificing a bit of memory to avoid the boxing.

sdanzig
  • 4,510
  • 1
  • 23
  • 27
  • This is Java, not C++ – Basilevs Oct 24 '13 at 04:09
  • I don't do C++. When I say comparing memory location for equality, I mean you're comparing if the objects are the same instance in memory... taking up the same space, because it's the same thing. – sdanzig Oct 24 '13 at 04:14
  • -1 What you are trying to formulate here seem to be about reference equality. And no, reference equality is never checked in my class. – Basilevs Oct 24 '13 at 04:26
  • The static arrays.equals method does a check on the individual elements in the object array, but it's still a homogenous object array, not an array of B and then C. However, I did forget about polymorphism, which would apply here. Oops. – sdanzig Oct 24 '13 at 09:35