Feedback on Records in JavaFX

[Brian Goetz]

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.



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