Feedback on records (JEP 359)

[Remi Forax]

Hi Anthony,
a lot of questions :)

----- Mail original -----
> De: "Anthony Vanelverdinghe" <anthonyv.be at outlook.com>
> À: "amber-dev" <amber-dev at openjdk.java.net>
> Envoyé: Samedi 25 Avril 2020 17:02:44
> Objet: Feedback on records (JEP 359)

> Hi
> 
> The following is feedback on my initial experience with records:
> 
> 1) a request: besides Brian's datum write-up [1], it would be useful to
> have a more practice-oriented document, similar to the one for text
> blocks [2]. For example, one thing I'd like such document to expand on,
> would be migration compatibility between records and classes (i.e. if I
> expose a record in my public API, how will that impact my ability for
> evolving it, e.g. converting it into a class?). This would ideally also
> include a FAQ which consolidates the wisdom from the mailing lists (e.g.
> "Why is the canonical constructor specified to be public?" [3], "Why
> aren't accessor methods generated as final?" [4], etc.).

Are you proposing yourself to write such FAQ ?

> 
> 2) a proposal: a method `Stream<IndexedElement<T>> indexed()` on Stream,
> which wraps each Stream element in a `record IndexedElement<T>(long
> index, T t)`
> This would allow for a natural, concise way to access indices (cf. e.g.
> [5]).

It's under consideration in one form or another but under the project Valhalla,
the introduction of inline class and reified generics will help a lot.

> 
> 3) a proposal: if a non-canonical constructor is defined, it should also
> be required to define the canonical constructor
> This is a trivial one-liner to add, and it reminds people that the
> canonical constructor is:
> a) present (unlike classes, where, if you define a constructor yourself,
> the implicit constructor doesn't exist)
> b) public (if there's a single private constructor, I regularly forget
> that there's still a public constructor as well)

We are revising the spec so a canonical constructor of a record in java 15 will has the visibility of the record by default.
And if you will full encapsulation, a record is not what you want, you want a class. A record give up encapsulation for convenience.

> 
> 4) an observation: I noticed that the ease with which records can be
> created, might lead to recorditis, where e.g. you start out with:
> 
> record Episode(int season, int number, String title) {}
> 
> and you end with:
> 
> record Season(Id id, EpisodeCount numEpisodes) {
>     record Id(int number)
>     record EpisodeCount(int numEpisodes) {}
> }
> record Episode(Id id, Title title) {
>     record Id(Season.Id season, int number) {}
>     record Title(String title) {}
> }

It's usual when you have a new toy/tool to use it every where until you start to understand where it makes sense and where it should be avoided.
Remember that phase where every classes of your code was a design pattern :) 
BTW, the code above will soon not compile anymore because the record component type will be resolved from outside the record instead of from inside it.

> 
> which brings me back to my first point: I'm hoping that a document with
> best practices and advice would give me some guidance on when to stop.
> 
> 5) an observation/proposal: I need to remind myself sometimes that
> record components are nullable. Would it be an option to introduce a
> `nullable-record` keyword, where `record` would not allow any of its
> components to be null, whereas `nullable-record` would?

It's not directly related to record, yes, in Java there is no way (or too many ways/annotations) to specify nullability.
There were discussion at last JVM Language Summit to try to build a consensus around what the exact semantics should be, so it's moving ... slowly as usual.

> 
> 6) a proposal: there's a recurring occurrence of boilerplate in my
> records, which could be eliminated by introducing a `sorted-record`
> keyword. This would work as follows:
> 
> sorted-record YearMonth(Year year, Month month) {}
> 
> is equivalent to:
> 
> record YearMonth(Year year, Month month) implements Comparable<YearMonth> {
> 
>     private static final Comparator<YearMonth> NATURAL_ORDER = Comparator
>         .comparing(YearMonth::year,
> Comparator.nullsLast(Comparator.naturalOrder()))
>         .comparing(YearMonth::month,
> Comparator.nullsLast(Comparator.naturalOrder()));
> 
>     @Override
>     public int compareTo(YearMonth o) {
>         return NATURAL_ORDER.compare(this, o);
>     }
> 
> }
> 
> on the condition that each record component either implements
> Comparable, or is an array type whose component type implements Comparable.
> Analog to YearMonth (yes, I'm aware that YearMonth et al. are already in
> the JDK ;)), there exist many examples, where a natural order on the
> instances is achieved by comparing all components, in order, from left
> to right. For example `Season.Id` and `Episode.Id` given above,
> `Type(String packageName, String typeName)`, `Error(String file, int
> line, int column)`, `Person(String lastName, String firstName)`, ...
> This is similar to how Enum also implements Comparable according to what
> one would naturally expect.
> This is tedious to write, but seems relatively easy for a compiler to
> generate.
> This would also nudge developers to declare record components in a
> consistent fashion. For example, in Europe, `LocalDate(int day, int
> month, int year)` would be pretty natural, but everyone would write
> `sorted-record LocalDate(int year, int month, int day)`.
> W.r.t. naming, "sorted-record" would fit naturally with
> SortedMap/SortedSet, Stream::sorted, etc.
> And the use of a hypenated keyword would allow `sorted-class` as well
> (though in my experience it's much less common for a class to only
> contain instance variables that implement Comparable).
> There could also be an Xlint warning for a sorted-record that contains
> components which have a type whose Comparable implementation is known
> not to be compatible with Object::equals (though this would only be
> verifiable for JDK classes, with BigDecimal as the notable example).

Brian already answer to that one.
I will add that i have found that exposing a Comparator is usually more flexible than implementing Comparable,
it's again a case where using delegation is better than using inheritance because you have a better composability.

> 
> Thanks in advance for any feedback.
> 
> Kind regards,
> Anthony

regards,
Rémi

> 
> [1] https://cr.openjdk.java.net/~briangoetz/amber/datum.html
> [2] https://cr.openjdk.java.net/~jlaskey/Strings/TextBlocksGuide_v10.html
> [3]
> https://mail.openjdk.java.net/pipermail/amber-spec-observers/2019-November/001883.html
> [4] https://mail.openjdk.java.net/pipermail/amber-dev/2020-April/005888.html
> [5]
> https://stackoverflow.com/questions/18552005/is-there-a-concise-way-to-iterate-over-a-stream-with-indices-in-java-8