Records and mutable components (was: Updated document on data classes and sealed types)

[Brian Goetz]

Splitting this into two topics.  

> 
>     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. :-(

I get where you’re going.  (Meta observation: discussions like this just underscore the importance of having a clear, crisp “what are records for” message, which plays into the other question.)  

You could call this “undermining”, or you could call this “pay to play.”  A record with lots of immutable components, and one pesky array, will still benefit form records, just a little bit less.  

Here’s a valid record:

     record MutableArrayPair(int[] as, int[] bs) { }

Before you recoil in horror (“your mutability is showing, mon dieu!”), let’s remember that the requirement to encapsulate mutability is not a law of nature, as much as a statement of which boundaries you care about.  Code that trusts its clients is perfectly justified in writing the above record, and surely would not want to be shut down by the rules.  (Note this is the semantics you’d get with public final fields, or the equivalent.)  So I think the above is a valid design choice for records, which is responsible in some situations and irresponsible in others.  And we get the nice invariant that deconstruction + reconstruction is an identity under equals(), because both the accessors and the equals method work the same way.  

Here’s another valid record:

     record EncapsulatedArrayPair(int[] as, int[] bs) {
          public EAP { as = as.clone(); bs = bs.clone(); }
          public int[] as() -> as.clone();
          public int[] bs() -> bs.clone();
          public int hashCode() -> Objects.hash(Arrays.hashCode(as), Arrays.hashCode(bs));
          public boolean equals(Object o) -> 
               o instanceof EAP eap && Arrays.equals(as, eap.as) && Arrays.equals(bs, eat.bs);
    }

That’s a valid record too, and shares the nice identity with its leaky cousin — that deconstruction + reconstruction yields an equals() instance.  

Now, you say: “but that’s a stupid record, you reimplement almost all the methods!”  Which would be true in Billy-World, where records are only for boilerplate reduction.  But records are a semantic statement — that their API is coupled to their representation (and hence, we get pattern-friendliness, and that certain API elements have useful invariants.)  Plus, we haven’t redeclared _all_ the members, and if there were other fields, that would be even less true.  

> 
> 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.

I think mismatch is too strong; the interaction between mutable components (especially those that don’t admit a nice immutable wrapper like Collections::unmodifiableXxx) and records causes friction, which requires the user to do some extra work to make up the difference.  But this isn’t “glass 100% empty”, it’s “glass less full than it would be in a perfect world.”  

I kind of like the “pay to play” nature; if you want to leak mutability, you can; if you want to plug it, you can, but you might be irritated at the cost (or not!), and if you are, you might seek alternate strategies (like more immutability, immutable wrappers, etc.)  
> 
> 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.

So, I agree with the world you want to get to.  And I think natural laziness will help pull users there too; the opportunity to do some fixup in one place seems more attractive than N places.  But I think _prohibiting_ these things — which I was initially attracted to — does not lead us to a stable place.  I think this one is better to encourage through carrot (e.g., value array wrapper classes, freezable arrays) than stick (records + arrays = leaked mutability).  

> 
> 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.

In part, we’re comfortable denying mutable fields because we _can_; the language has a concept of field mutability.  It does not, for better or worse, have a concept of deep immutability.  So attempts to discourage deep mutability (such as, by making it more dangerous when it comes in contact with records) feels like the wrong end of the lever.  

> 
> 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.

I think we’re in agreement; the “wrong” way to do it is also really hard to implement correctly.  

Looking ahead (please, let’s not bikeshed this now), I want to take this idiom (“bound constructors”) from records and eventually make it possible to declare a “constructor whose parameters are bound to fields”, and yield the same sort of “we’ll fill in the error-prone boilerplate for you” result.  


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/amber-spec-experts/attachments/20190309/a94f3938/attachment-0001.html>