Hi,
I think that, to avoid confusion, there should be no special treatment
of arrays. They should be treated as any other reference. Why?
Reason #1:
Suppose we have:
value U {
int[] array;
U(int[] array) {
this.array = array;
}
}
value V {
Object obj;
V(Object obj) {
this.obj = obj;
}
}
int[] a1 = new int[] { ... };
int[] a2 = new int[] { ... };
U u1 = U(a1);
U u2 = U(a2);
V v1 = V(a1);
V v2 = V(a2);
I think that it would be very confusing for the following to no hold for
any a1 and a2:
assert (u1 == u2) == (v1 == v2);
assert u1.equals(u2) == v1.equals(v2);
If you want to support above assertions and still treat arrays
differently from non-array references, then a runtime dispatch dependent
on a runtime reference type will be needed. If you do that then you have
specified an operation performed on components of a value that is not
expressible in language with two locals holding the values of the those
component(s) short of a library function call.
Reason #2:
It's not difficult to create a standard reusable value type wrapping a
single array with suitable equals() override:
value Array<any T> {
private T[] array;
Array(int length) {
array = new T[length];
}
@Override public boolean equals() {
...
}
}
... and use it instead of plain array(s) as components of other value
or reference types, with no runtime overhead compared to plain arrays.
For this to work then default .equals() for value types should:
- compare primitive components with ==(p, p) (modulo float/double
variations for which I don't have an opinion yet)
- compare reference components with .equals()
- compare value components with .equals() - yes, this gives the most
intuitive result and encapsulates equality into the classes of the
components.
For == operator on value types then I don't know. Perhaps it should be
the "bit-wise" comparison of components or, a John would say,
copy-equivalence.
That's simple to grasp and explain and is also usable.
Regards, Peter
On 05/17/2016 04:56 PM, Brian Goetz wrote:
So, its worth pointing out here that the only disagreement is on a
pretty cornery (and fundamentally messy) case -- arrays as components
of a value.
I think there's no dispute over:
- primitives components compared with ==(p,p)
- value components compared recursively with ==(v,v)
- non-array reference components compared recursively with .equals()
So the only remaining question is what to do with arrays (and, for
equals vs deepEquals, these differ only when arrays are nested within
arrays within values -- very much a corner case). Options include:
- reference equality / Object.equals()
- Arrays.equals on primitive / value arrays, but reference equality
on object arrays
- Arrays.equals
- Arrays.deepEquals
The first says "are they the same array". The second lifts equality
onto components known to be immutable, but punts on mutable array
components. The third lifts .equals() equality onto
potentially-mutable array components; the third goes farther and
recursively lifts this definition of equality onto elements that are
actually arrays (which must be done reflectively -- see the nasty code
below which is called on each element.)
In what cases will you have an array nested in an array as a value
component? I'm having a hard time thinking of any -- can you dig up
examples?
The cases where I'd have an ordinary array in a value fall into two
(opposite) categories:
- Where the array is effectively immutable and holds the state of the
value, such as a string-like value class (holding a char[]) or a
BigDecimal-like value class (holding a long[]);
- Where the value is behavior like a cursor into an array.
In the first case, two BigDecimal are equals() if they have the same
array contents, so at first we might think Arrays.equals() is what we
want (and same for the String case.) But if I were coding a
BigDecimal value, I wouldn't want this default anyway -- since I want
to treat 1.0 and 1.00 as equal, which the default won't do. So while
the default is OK, I'm still going to override it (and similarly for
String), because Arrays.equals still produces false negatives.
In the cursor case, we definitely want to compare the arrays by
reference -- two cursors represent the same thing if they point to the
same element in the SAME array, not equivalent arrays. And, same with
an Optional<Foo[]>.
So, grading the possibilities:
- reference equality -- produces right answer for cursor, produces
wrong answer for BigDecimal/String cases.
- Arrays.equals -- produces wrong answer for cursor, produces
tempting-seeming but still wrong answer for BigDecimal/String.
So both cases produce the wrong answer for the value-like uses of
arrays -- and the user has to override the default anyway. The only
one that produces the right answer in any case is reference equality
(which is .equals()). Maybe there's more cases, but these are the
ones that come immediately to mind.
For arrays, it seems that trying to read the user's mind here is a
losing (and at the same time, expensive) game. Arrays are what they
are; trying to turn them into acts-like-lists when you don't know the
semantics of the array in their enclosing value seems pretty
questionable. Which leads me to prefer the following simpler default
for equals() on values:
- compare primitive and components with ==
- compare reference components with .equals()
Do you have data on what percentage of your AutoValue objects contains
arrays?
Secondary question: to what degree is the "if you don't like the
default, write your own" mitigated by helpers like Guava's
Objects.equals? (It does kind of suck that if you disagree with the
default treatment of one field, you have to rewrite the whole
equals/hashCode methods.)
FYI, nasty and expensive method called on every element by deepEquals:
static boolean deepEquals0(Object e1, Object e2) {
assert e1 !=null;
boolean eq;
if (e1instanceof Object[] && e2instanceof Object[])
eq =deepEquals ((Object[]) e1, (Object[]) e2);
else if (e1instanceof byte[] && e2instanceof byte[])
eq =equals((byte[]) e1, (byte[]) e2);
else if (e1instanceof short[] && e2instanceof short[])
eq =equals((short[]) e1, (short[]) e2);
else if (e1instanceof int[] && e2instanceof int[])
eq =equals((int[]) e1, (int[]) e2);
else if (e1instanceof long[] && e2instanceof long[])
eq =equals((long[]) e1, (long[]) e2);
else if (e1instanceof char[] && e2instanceof char[])
eq =equals((char[]) e1, (char[]) e2);
else if (e1instanceof float[] && e2instanceof float[])
eq =equals((float[]) e1, (float[]) e2);
else if (e1instanceof double[] && e2instanceof double[])
eq =equals((double[]) e1, (double[]) e2);
else if (e1instanceof boolean[] && e2instanceof boolean[])
eq =equals((boolean[]) e1, (boolean[]) e2);
else eq = e1.equals(e2);
return eq;
}
On 5/17/2016 1:06 AM, John Rose wrote:
On May 16, 2016, at 8:29 PM, Kevin Bourrillion <[email protected]> wrote:
And by "deep equals" I mean equivalent behavior to
https://docs.oracle.com/javase/8/docs/api/java/util/Arrays.html#deepEquals-java.lang.Object:A-java.lang.Object:A-
at least that is what AutoValue has been doing.
To the extent I understand this, it seems wrong: deepEquals treats
arrays as if they are lists, which is an abstraction shift. Surely
you aren't suggesting cracking a component reference in a subfield
and treating its object as a List of its components, and so on
recursively? Because that's what deepEquals does.
Or do you mean that, just as deepEquals avoids using op==(ref,ref) on
array components, so "deep equals" should avoid using op==(ref,ref)
on value components?
You can avoid op==(ref,ref) by replacing it by a call to
ref.equals(ref), or you can avoid op==(ref,ref) by cracking the refs
and recursing. That latter breaks an abstraction, so I think we
agree it's bad, but *that* is a more "equivalent behavior" to
Arrays.deepEquals.
— John
