Updated document on data classes and sealed types

[Kevin Bourrillion]

On Fri, Mar 8, 2019 at 1:04 AM Brian Goetz <brian.goetz at oracle.com> wrote:

For a well-behaved record R(x,y) and an instance `r`, it seems pretty
> reasonable to expect that new R(r.x(), r.y()) equals r.  If we have
>
>      record KevinHatesLife(int[] ints) { }
>
> and we take the default accessor and equals, this will be true, but we’ll
> be leaking our mutability.  There exist cases where that’s OK and desired,
> but there are cases when we want to not be leaky.  So we override the
> ints() accessor to clone on the way out.  But now, when we
> deconstruct-and-reconstruct, the new KHL is not equal to the old one.  To
> restore the desired equality semantics, we want to override equals as
> follows:
>
>     boolean equals(Other o) {
>         return (o instanceof KevinHatesLife(var is)) &&
> Arrays.equals(ints, is);
>     }
>
> (and of course the same for hashCode.)  Seems mean to not let you define
> equality in this way.
>
> Say it with me: “DAMN YOU, PERVASIVE MUTABILITY!"
>

Let me see if I have this right. If you want to use an array field, and you
are conscientious and want to avoid bugs, then

1. You need constructor boilerplate scaling with the number of array fields
2. You need accessor boilerplates caling with the number of array fields
3. You need to completely implement equals, hashCode, and toString yourself
if you have even *one* array.

The pattern here is how tidily it seems to undermine the benefits of
records. :-(

And other mutable types may not share #3, but do #1 and #2.

In other words, the records design simply does not play well with mutable
types, just as it doesn't with the fields being non-final. They are a
mismatch.

But here's my real question. Even if we enable users to do these
workarounds - is this even the remedy that we want to recommend in our
documentation? There's another remedy that I think is strictly better: *get*
yourself an immutable type to use, even if you have to create a wrapper
yourself. It's a small investment, it makes accessors as cheap as expected,
and the very next time you want to use this field type in a record you'll
already be very glad. And sometimes you will discover that a suitable
immutable type already exists. Maybe the ones people create for this reason
will get shared more. This is a pretty good picture of the world imho.

This is *not* a religious argument about "immutable good mutable bad". And
this is not pretending that we can uninvent mutability. Mutability has its
places, many of them. This is asking whether records really can be one of
those places. Are we *really gaining anything* with this approach to trying
to accommodate it? We're already comfortable denying mutable fields.


Yes!  While it may seem more “efficient” to just write to the field
> directly, it would make things much more complicated.  If our ctor was
>
>      Foo {
>           if (x < 0)
>               this.x = 0;
>      }
>
> now on exit from the ctor, this.x is neither DA nor DU, so the compiler
> would have to generate some pretty nasty code to replicate the conditions
> under which it would want to do the assignment.  So either the user-written
> ctor always writes the field (DA), or never does (DU) — or it’s an error.
>

Well, I don't care about nasty compiler code; it's still the right way to
do it. :-) So that's a note for the next version of the doc.

The stricture against derived fields was probably the hardest choice here.
>> On the one hand, strictly derived fields are safe and don’t undermine the
>> invariants; on the other, without more help from the language or runtime,
>> we can’t enforce that additional fields are actually derived, *and* it will
>> be ultra-super-duper-tempting to make them not so.  (I don’t see remotely
>> as much temptation to implement maliciously nonconformant accessors or
>> equals methods.)  If we allowed additional fields, we would surely have to
>> lock down equals/hashCode.
>
>
> I still want to understand what the scenario we're worried about here is.
> Whether the value is computed later using a "lazy fields" feature or
> eagerly in the constructor, only the record's state is in scope, and sure,
> people *can* shoot themselves in the foot by calling out to some static
> method and getting some result not determined by the parameters, but why is
> this worth worrying about? Do you have an example that's both dangerous and
> tempting? (Sorry if you've said it before.)
>
> If we did allow them, the next thing people (Alan already did, and you
> made this same comment in an earlier round) would ask is whether they can
> be mutable — so that derived fields can be lazily derived,  Now, records
> are a combination of a “true record" plus an unconstrained bag of mutable
> state.
>

My question did actually exclude this option. We should tell those people
no.


 And what do you think the chances are that this state won’t make it into
> equals/hashCode semantics?
>

If it's derived, it doesn't hurt that much; if it's not, why are they
working so hard to not make it a regular record field?
This is part of what I'm talking about when I say "sure, they *can* shoot
themselves in the foot". What is a realistic example we are worried about?


Now, we’ve completely lost our grasp on the semantic constraint — that a
> record is “just” its state.
>

Well, it's still just its *real* state.


 We could try to put the toothpaste back in the tube by clamping down on
> the ability to override equals/hashCode, but now the previous example rears
> its head again.
>
> Worse, it does a lot of damage to the mental model of what records are
> for. We know they’re not about writing a class with fewer lines of code
> (though that’s an advantage), they’re about transparent carriers for a
> defined unit of state.  But, if users routinely see records in the wild
> with lots of extra state stapled to the side, maybe even affecting
> equals/hashCode, this design center is going to be harder to see (this
> ultimately leads to a feedback loop, where users who can’t understand what
> the feature is for will demand more features that are out of line with its
> design center, further obscuring the design center.)
>

Just saying that I don't share this concern. And I get concerned about
everything. :-) Derived state is common and not complicated. I think it's
just the fear of this being abused for non-derived state that we're talking
about. Isn't it just another case like "people can pass side-effecting
functions to map()"?

I'm trying to figure out if there's somewhere I'm being logically
inconsistent with myself, because it feels weird to be simultaneously
arguing for allowing something you don't want to allow and for disallowing
something you do want to allow. :-)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/amber-spec-experts/attachments/20190308/f02b8737/attachment-0001.html>