Should I use Objects.equals() on each element rather than Arrays.equals() when the array is composed of fields in my class?

Question

I'm writing a tuple utility, and so that I have the type safety of generics, I have a TupleN classes where N is the number of elements (and thus type parameters) it has, all of which inherit from an abstract class Tuple. Here's (a shortened version of) my Tuple class, as well as the Tuple2 class (all the other TupleN classes follow the same pattern).

Tuple.java

public abstract class Tuple {

  public abstract int arity();

  public abstract Object get(int index);

  public Object[] toArray() {
    ArrayList<Object> list = new ArrayList<>();
    int arity = arity();
    for (int i = 0; i < arity; i++) list.add(get(i));
    return list.toArray();
  }

  @Override
  public boolean equals(Object o) {
    return o instanceof Tuple && Arrays.equals(toArray(), ((Tuple) o).toArray());
  }

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

  @Override
  public String toString() {
    return String.format("(%s)", String.join(", ", toArray().stream().map(t -> Objects.toString(t)).toList()));
  }

}

Tuple2.java

public class Tuple2<T0, T1> extends Tuple {

  public final T0 t0;
  public final T1 t1;

  public Tuple2(T0 t0, T1 t1) {
    this.t0 = t0;
    this.t1 = t1;
  }

  @Override
  public int arity() {
    return 2;
  }

  @Override
  public Object get(int index) {
    switch (index) {
      case 0: return t0;
      case 1: return t1;
      default: throw new IndexOutOfBoundsException(index);
    }
  }

  @Override
  public Object[] toArray() {
    return new Object[] {t0, t1};
  }

  @Override
  public boolean equals(Object o) {
    if (o instanceof Tuple2) {
      @SuppressWarnings("unchecked")
      Tuple2<T0, T1> other = (Tuple2<T0, T1>) o;
      return Objects.equals(t0, other.t0) && Objects.equals(t1, other.t1);
    }
    return false;
  }

  @Override
  public String toString() {
    return String.format("(%s, %s)", t0, t1);
  }

}

I was going to override both equals and hashCode in my TupleN classes for efficiency's sake, but I realized that my implementation of hashCode would be return Objects.hashCode(t0, t1);, which (from the docs) is the same as return Arrays.hashCode(new Object[] {t0, t1});, which (what with my overrided toArray method) is exactly what the default implementation does. After that, I realized that my override of equals is pretty much just the default implementation of equals, except where Arrays.equals had been "unfolded." My question is this: is that "unfold" sufficiently more efficient for it to be worth it to write out the override (which isn't too long for a Tuple2, but when it comes to a Tuple7, that sounds like a lot of boilerplate code), or would it be okay to just use the default implementation?

EDIT:

While I'm at it, I may as well ask the same about toString, though I imagine the default implementation of that takes more effort (what with streams and such).

Answer 1

Objects.hashCode calls Arrays.hashCode, but its use as varargs method duplicates the array creation code at the caller’s side, so the outcome might be even worse than your base class’ method. There might be optimizations on the JVM side, but that’s pure speculation and it would be pointless to create more code without knowing the actual implication to the performance.

I think, it’s worth to go the route of implementing an equals and hashCode method by hand here, if you do it only once for the base class without the need to create specializations in the subclasses, as all you need for a copy-free implementation is already there, namely arity() and get(int). The implementation basically does the same as AbstractList, which you do not want to extend/inherit for the sake of a clean API (without unsupported methods).

A toString() method based on the Stream API should be sufficient for most purposes, though, even here there is no need to create an array first, not to speak of collecting into a List, just to call String.join, when you can collect into a string of the right format in the first place:

public abstract class Tuple {
    public abstract int arity();
    public abstract Object get(int index);

    public Stream<Object> elements() {
        return IntStream.range(0, arity()).mapToObj(this::get);
    }

    public Object[] toArray() {
        return elements().toArray();
    }

    @Override
    public String toString() {
      return elements().map(Objects::toString).collect(Collectors.joining(", ", "(", ")"));
    }

    @Override
    public boolean equals(Object o) {
        if(o == this) return true;
        if(!(o instanceof Tuple)) return false;
        Tuple t = (Tuple)o;
        int n = t.arity();
        if(n != arity()) return false;
        for(int i = 0; i < n; i++) if(!Objects.equals(get(i), t.get(i))) return false;
        return true;
    }

    @Override
    public int hashCode() {
        int result = 1;
        for(int i = 0, n = arity(); i < n; i++)
            result = 31 * result + Objects.hashCode(get(i));
        return result;
    }
}

The effort spend in the base class methods pays off at the subclasses which do not need to provide optimized versions

public final class Tuple2<T0, T1> extends Tuple {
    public final T0 t0;
    public final T1 t1;

    public Tuple2(T0 t0, T1 t1) {
      this.t0 = t0;
      this.t1 = t1;
    }

    @Override
    public int arity() {
      return 2;
    }

    @Override
    public Object get(int index) {
        // starting with Java 9, you may consider Objects.checkIndex(index, arity());
        if((index|1) != 1) throw new IndexOutOfBoundsException(index);
        return index == 0? t0: t1;
    }
}

public final class Tuple3<T0, T1, T2> extends Tuple {
    public final T0 t0;
    public final T1 t1;
    public final T2 t2;

    public Tuple3(T0 t0, T1 t1, T2 t2) {
      this.t0 = t0;
      this.t1 = t1;
      this.t2 = t2;
    }

    @Override
    public int arity() {
      return 3;
    }

    @Override
    public Object get(int index) {
        switch(index) {
            case 0: return t0;
            case 1: return t1;
            case 2: return t2;
            default: throw new IndexOutOfBoundsException(index);
        }
    }
}

etc

Beyond that, only provide overriding methods if a profiling tool revealed that there truly is a bottleneck at a certain place and also proves that the specialized implementation truly has a performance benefit over the general code.

Answer 2

Yes, Arrays.equals() is enough for this. You do not need to check each one yourself.

The way I see it, you aren't reusing a lot of your things in your abstract class. You should be able to keep your own array (Object[]) and let your abstract class do most of the work.

public abstract class Tuple {
    private final Object[] data;

    protected Tuple(Object... data) {
        this.data = data;
    }

    protected final Object[] getArray() { return data; }
    public final Object[] toArray() { return Arrays.copyOf(data, data.length); }

    public Object get(int index) {
        return data[index]; // You need to do your boundary checks
    }

    // Same hashcode and equals methods
}

public class Tuple2<T0, T1> extends Tuple {
    public final T0 t0;
    public final T1 t1;

    public Tuple2(T0 t0, T1 t1) {
         super(t0, t1);
         this.t0 = t0;
         this.t1 = t1;
    }
}

Answer 3

Arrays.equals() will call your equals() method, and do the loop for you.

Ditto toString(). In this case as long as you're happy with the default output format of Arrays.toString() there is no need to look further.