Feedback on Records in JavaFX

[Remi Forax]

----- Mail original -----
> De: "Brian Goetz" <brian.goetz at oracle.com>
> À: "Nir Lisker" <nlisker at gmail.com>, "amber-dev" <amber-dev at openjdk.java.net>
> Envoyé: Jeudi 7 Mai 2020 16:16:31
> Objet: Re: Feedback on Records in JavaFX

> I thought I replied when you sent it, but apparently not.
> 
> Your list of "almosts" corresponds almost exactly to the exploration we
> did during the design process, enumerating examples of patterns which
> make a class unsuitable for being a record.  At some level, it is a
> positive validation that your list did not contain anything that was
> unknown during the design.
> 
> For each of these, we considered -- sometimes agonized over -- what it
> would take to bring these into the fold.  And for each, they either took
> records away from their design center, or greatly increased the
> complexity of the feature, or made it more confusing to users what
> records were.
> 
> Looking at records through the lens of "how many of my classes can I
> convert", while tempting, is not necessarily the right way to judge the
> feature.  Instead, perhaps we should ask ourselves: "if I had records
> when I wrote this, could I have used them profitably?"  (When enums came
> out, I had very few classes lying around that I could just convert to an
> enum -- but since then, I've found zillions of uses for enums.  (And
> yes, I've also found many "almost" uses for enums, which is sad.  I
> expect it will be the same way with records.))
> 
> To score your list against the design goals (but please, people, this is
> not an invitation to reopen the debate over the design goals):
> 
> 1/2 (unwanted accessors.)  These basically say that the class falls
> outside the design center -- a record is a nominal tuple. Languages with
> tuples don't give you a way to say "but item #3 is private"; that's not
> what tuples are.
> 
> 3 (extending abstract classes.)  This is one of the few on the list that
> we could conceivably have covered.  We'd have to have made `Record` an
> interface, and impose some severe restrictions on the superclasses,
> though, and at the time, the complexity of this seemed out of line with
> the benefits.  (The abstract classes would not be allowed to have
> fields, which you might find too harsh a restriction.)  Since then,
> however, similar work in Valhalla has identified what a "stateless"
> abstract class looks like, so its possible we can revisit this before
> finalizing the feature, but the incremental benefit still seems thin.
> 
> In the examples you had with abstract classes, did they mostly have
> fields, or no?
> 
> 4 (mutability.)  This was obviously the most inconvenient restriction,
> and you could tie yourself in knots trying to accommodate them (which I
> did, for a while.)  In the end, though, we concluded that there was no
> intermediate point between the current design point and "anything goes"
> that was either satisfying or stable.
> 
> If you allow mutable state, it is almost inevitable that you will also
> want to exclude some subset of fields (maybe all of them, but maybe not)
> from the canonical constructor, and also exclude some (but a possibly
> different one!) subset of them from the equals/hashCode computation.
> Which means that you would need syntactic surface for sorting components
> into various buckets (whether they go in the constructor or not, whether
> they are part of equals or not, whether they are part of toSTring or
> not, whether they get getters, whether they get setters, etc). These
> each clutter the code, and the feature essentially degenerates to being
> a syntax-only, macro generator.  (And if you look at all the options
> that tools like Lombok have grown over the years, we can predict there
> would be an infinite stream of calls for "Can you add this code
> generation feature.")
> 
> So while "can I just have mutability" sounds like a simple request, it
> essentially drives you towards building a very different feature, and
> one that arguably does not belong in the language.
> 
> 5 (caching.)  This is one we have a lot of sympathy for; this is
> entirely within the design spirit of records.  We looked for a way to do
> this, but all the things we looked at either would slide us back into
> the hell outlined above in (4), or really wanted to be a general feature
> for all classes, not just records.  It is possible that one of these
> approaches might reappear as a general feature (e.g., lazily cached
> fields), and if they do, we would surely consider if records could play
> in the fun.
> 
> 6 (internal state.)  This is another flavor of 4/5, where there is some
> state that is managed by the record but not part of the "record state".
> And again, we looked very hard at whether this could be accomodated, but
> again, there was no stable intermediate point that retained any of the
> semantic benefits of records.  Such internal state would invariably want
> to creep into the calculation of hashCode/equals, for example.  In the
> end, it was too far from the design center of "nominal tuple."
> 
> 7 (public API.)  This one goes back to the top point, which is that if
> you evaluate records by "can I convert my existing classes to records",
> you get a different evaluation than "could I have profitably used
> records when writing this class."  Yes, there will be existing public
> API that can't migrate.  But as you build new public API, you will
> surely consider "can I make this a record."
> 
> 
> Apologies again for not responding.  As I said, we're sympathetic to all
> these issues, and effort was expended attempting to accomodate some of
> them.  In fact, it was the efforts expended trying to bring these into
> the fold that brought us to a better understanding of what the stable
> design center was -- nominal tuples.

In term of design, there is still a possible intermediary step, we have talked a little bit,
in term of syntax with records, we have introduced the record keyword and the parenthesis syntax at the level of class declaration,
one good question is what the parens syntax means for a class or an enum.

By example,
  class Grid(int width, int height) {
    ???
  }

This can be an intermediary point in term of design in between a class and a record, this can also be the start of pointless discussions :)
Better to wait when enough people have used records.

Rémi

> 
> 
> 
> On 5/6/2020 8:23 PM, Nir Lisker wrote:
>> Hi,
>>
>> Was this missed or is it not interesting?
>>
>> - Nir
>>
>> On Sun, Apr 26, 2020 at 5:34 AM Nir Lisker <nlisker at gmail.com> wrote:
>>
>>> Hi,
>>>
>>> In order to both prepare JavaFX for the records feature and provide
>>> feedback on the feature here, I created a branch [1] in my JavaFX fork
>>> where I test records. I surveyed many (but far from all!) JavaFX classes
>>> and tried to replace current implementations with records wherever I could.
>>> I was not very careful with checking toString, hashCode and equals existing
>>> implementation, and assumed, for the purpose of this test, that the default
>>> implementation does what the original implementation is supposed to do. I
>>> will also note that I did not do any design changes, though some of the
>>> classes could probably have been refactored to a more sensible design from
>>> which a record could be extracted.
>>> The branch contains some build related files like .classpath files for
>>> Eclipse that can be ignored. The code changes are either record
>>> implementations, or changes to classes that now use records and needed to
>>> change the accessor name they call.
>>>
>>> I hope that this can serve as a concrete example of migration of a large
>>> production library, and would like your feedback on the changes as well.
>>>
>>> Additionally, here is a list of classes that "almost" fit a record for
>>> various reasons (you can use the GitHub file search to find them).
>>>
>>> 1. Generation of unneeded accessors for private classes
>>> Example classes:
>>> modules/javafx.fxml/src/main/java/com/sun/javafx/fxml/expression/Expression.java
>>> (#Token)
>>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/SelectorPartitioning.java
>>> (#PartitionKey)
>>> modules/javafx.web/src/main/java/com/sun/webkit/network/DateParser.java
>>> (#Time)
>>>
>>> These classes behave like records, but accessors will be generated for
>>> fields that do not need them. Inner classes' private fields are accessible
>>> to the enclosing class, so many times there is no need for accessors. This
>>> is a minor point as they can just be ignored, but if there is a cost to
>>> this, I don't want to pay it.
>>>
>>> 2. Generation of unneeded accessors for non-private classes
>>> Example classes:
>>>
>>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/CascadingStyle.java
>>> modules/javafx.graphics/src/main/java/javafx/css/Match.java
>>> modules/javafx.web/src/main/java/com/sun/webkit/ContextMenu.java
>>> (#ShowContext)
>>>
>>> These classes behave like records, but accessors will be generated for
>>> fields that are not meant to be exposed.
>>>
>>> 3. Extending abstract classes
>>> Example classes:
>>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/FontFaceImpl.java
>>>
>>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/PseudoClassImpl.java
>>>
>>> These classes behave like records, but they extend an abstract class that
>>> does not have any fields. I remember a discussion for allowing this
>>> inheritance by making Record an interface,
>>>
>>> 4. Mutability
>>> Example classes:
>>> modules/javafx.graphics/src/main/java/com/sun/prism/BasicStroke.java
>>> modules/javafx.graphics/src/main/java/com/sun/javafx/geom/Vec2d.java (and
>>> similar VecXx)
>>>
>>> These classes behave like records, but they require mutable fields.
>>>
>>> 5. Caching hash code
>>> Example classes:
>>> modules/javafx.web/src/main/java/com/sun/webkit/text/TextBreakIterator.java
>>> (#CacheKey)
>>>
>>> This class behave like records, but caches the hashcode. This is useful in
>>> cases where hashcode computation is expensive. Since the fields are final,
>>> this value is *usually* constant, but in case of mutable reference types,
>>> their hashcode can change externally after they are passed to the record.
>>> This can be mitigated by defensive copying. There's a lot that can be said
>>> here, but it might seems reasonable to support this caching in some way
>>> (I'm waving my hands here).
>>>
>>> 6. Internal state
>>> Example classes:
>>>
>>> modules/javafx.web/src/main/java/com/sun/webkit/event/WCInputMethodEvent.java
>>>
>>> This class behaves like a record, but has an internal field that is not
>>> passed through the constructor. Notably, this class also does defensive
>>> copy of the array both in and out (using Arrays.copyOf and not clone as
>>> I've seen before).
>>>
>>> 7. Public API
>>> Example classes:
>>> modules/javafx.graphics/src/main/java/javafx/animation/KeyFrame.java
>>> modules/javafx.graphics/src/main/java/javafx/animation/KeyValue.java
>>> ("suffers" from point 6)
>>> modules/javafx.base/src/main/java/javafx/util/Pair.java
>>>
>>> These classes behave like records, but they are public API, so there could
>>> be migrations issues.
>>>
>>> * Additional note about accessors of boolean fields
>>> Example
>>> class: modules/javafx.web/src/main/java/javafx/scene/web/PopupFeatures.java
>>>
>>> This class uses boolean fields named "menu", "toolbar" etc. to denote if a
>>> menu or a toolbar exists. In this case, having a "hasX()" accessor makes
>>> sense while "x()" does not. In my implementation, I changed the field name
>>> to "hasX" so that the accessor names would match. I think that booleans are
>>> a special case. Most accessors retrieve a value like "width()" or "name()",
>>> but booleans usually answer an "is" or "has" question - "isEnlarged()" and
>>> not "enlarged()". Enforcing the current name of the accessor can
>>> *sometimes* lead to awkward namings.
>>>
>>> ### Conclusions
>>> There are not that many places where records can be used, though for these
>>> specific use cases they fit well (nominal tuples); many of them are inner
>>> classes. From the above list, some stray a lot from the idea of records
>>> (4), some not so much (1, 3).
>>> I would cautiously say that the design is too restrictive, especially when
>>> it comes to internal helper state (5, 6). I would think that, while
>>> a record component is given to the constructor and exposed through the
>>> accessor, other internal (final) fields that help with implementations
>>> would be useful. This reminds me of interfaces - while their public methods
>>> serve as contract, private methods were made valid just to help with
>>> internal implementation, and they are invisible to the contract.
>>> With regards to accessor visibility (1, 2), I know that there was (or is)
>>> a discussion about allowing to reduce their visibility to that of the class
>>> (not sure about the constructor). Sometimes there is no need for them at
>>> all.
>>>
>>> I hope that this concept of records can be expanded, either to specialized
>>> types for other similar use cases, or with changes from other features that
>>> work well with records, as records alone don't hit enough targets in my
>>> opinion. I intent to continue to update this branch as I work with whatever
>>> use cases I find, so by the time JavaFX can use records we would have a
>>> good starting point.
>>>
>>> Best,
>>> Nir
>>>
>>> [1] https://github.com/nlisker/jfx/tree/Records