Feedback on Records in JavaFX

[Nir Lisker]

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