Value equality

[Peter Levart]

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 <kevinb at google.com> 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
>