Feedback regarding default equals, hashCode and toString implementions for records
John Rose
john.r.rose at oracle.com
Mon Apr 27 19:15:07 UTC 2020
Brian’s answer is definitive. I can’t resist adding some comments
about mutability vs. encapsulation.
A record which contains one or more arrays may (or may not) be
intended to embody an immutable record of some array’s elements.
In that case, a completely clean encapsulation should not only tweak
equals/hashCode/toString to descend into the array’s component
structure, but it must also protect the array from modification after
the record has been created.
(This may be the case. Or it may not. The programmer has to choose.
I’m just saying it’s an important use case. See Brian’s point about
the language not being able to read the programmer’s mind.)
On Apr 27, 2020, at 7:52 AM, Brian Goetz <brian.goetz at oracle.com> wrote:
>
> In the end, we settled on the simpler alternative, which is to uniformly appeal to Object::equals, and advise users to take care when using arrays and records, and consider using List instead if content equality is what is desired.
In fact, List::copyOf can be used in the compact constructor to
ensure that clients of the record cannot change any element of
the record’s logical value.
record R(List<String> l, int x) {
public R {
//defensive copy to protect value
this.l = List.copyOf(l);
}
}
I think of this as the author of the record choosing to “draw a boundary”
around the record’s logical state which includes not only the components
(l, x), but also includes the elements of l (l.get(0), l.get(1), …).
As a bonus, List::copyOf is likely O(1) when repeated on its own output.
Once a list is immutable via copyOf, further requests to make it immutable
probably return the original list.
You can’t do this trick with arrays unless you defensively copy in
*both* the constructor and the accessor. Another reason we rejected
this feature in records is that it wasn’t a complete solution to
making array elements part of the record’s logical state, since naked
array elements are mutable by anybody that obtains them. That would
violate the clean encapsulation boundary I described. So a feature
which magically calls Arrays::equals in the record equals method will
unlock a second request for a magical feature which defensively copies
the array in the constructor and the accessor.
So the simplest way to supply both deep equals and full protection
against client mutations is for the record author to wrap the aggregate
in a List, not ask the language to read the user’s mind (as Brian put it).
This design doesn’t meet all possible follow-on requirements (see
P.S.s below, and maybe more “what-about” responses in this thread),
but it’s by far the simplest design for records, and covers follow-on
requirements fairly, at the cost of additional coding by the user,
whose mind the language refuses to read—and the men in black
won’t let us release that proprietary alien technology until at least
Java 42.
— John
P.S. What about a user who actually wants mutability of a component
array *and* the pretense by equals/hashCode/toString that the current
components of the array are part of the record’s (now mutable) logical
contents? That user could consider something like ArrayList::new in
the record constructor, instead of List::copyOf. That will present the
desired behavior.
P.P.S. What if the user says, “I don’t want extra objects, so I need
to have a real array component in my record”? That user should be
willing to pay the costs of obtaining that level of tuning. (Same
answer if the user for some reason loves arrays and wants an array
type in the record API.) The default behavior of records is in fact
oriented in that direction, except for the default behaviors of equals
etc. If the user *also* wants logical encapsulation of array elements
*and* no intermediate objects, then the user should expect to have to
do some more coding to meet both requirements. Performance and
encapsulation sometimes must be traded off against each other.
When Valhalla delivers, it will be easier to write a wrapper inline class for
the array which is more efficient than List::copyOf. In fact, List::copyOf
itself may evolve to use such a wrapper internally, lifting all boats.
P.P.P.S. What about primitives? Arrays efficiently collect
primitives, not lists. Sadly, this is a deep problem, to be addressed
in Valhalla, and not efficiently addressable as a point-fix in the
design of records. Use an array if you are performance sensitive, but
be ready to write more code. Maybe write yourself an int-array
wrapper auxiliary type for your record, and use that as a component.
P.P.P.P.S. What about varargs? Perhaps someone wanted arrays, not
lists, because the constructor was supposed to be variadic. This can
be covered (at a modest cost in extra boilerplate) by another
constructor:
public R(int x, String... ss) {
this(List.of(ss), x);
}
Should this boilerplate be removable also, somehow? Maybe, but that’s
a job for another day. Having to write out varargs constructors like this
is a fair price to pay, in order to fit varargs into a record which is designed
to protect array elements.
And there may be a path (not any time soon) to allowing Lists to play
directly as carriers for variable-arity method calls. See JDK-8182862
for some thoughts that wander in that direction.
More information about the amber-dev
mailing list