[records] Is null back?

[Remi Forax]

----- Mail original -----
> De: "Sascha Kohlmann" <Sascha.Kohlmann at misterspex.de>
> À: "amber-dev" <amber-dev at openjdk.java.net>
> Envoyé: Samedi 7 Mars 2020 13:40:20
> Objet: [records] Is null back?

> Hi to all,

Hi Sacha,

> 
> I hope this is the correct place for comments about Records.
> 
> I've translated a Domain Model of a current project into records. The model
> contains a couple of immutable value objects. These are excellently suited to
> be implemented as records.
> 
> My observations
> 
> Change from old classes to records is simple for standard use cases. Dealing
> with different constructors is possible. Checking values and adjusting them if
> necessary is possible. I found it not very intuitive to use "this.value" if I
> don't see the instance variable. This may change later when we've more
> experience with records.
> 
> Null collections can be replaced by empty collections, so that null must not be
> returned.

You can use List.copyOf()/Set.copyOf() in the compact constructor too.

> 
> With other null parameters, however, it is more difficult. I had got used to
> never giving back null since Java 8 if null is a possible input value. Instead
> I always return Optional<T>. This is only possible with records if I pass
> Optional<T> as parameter into the record. Nevertheless I have to check it again
> for null to replace it with Optional.empty() if necessary. And Optional<T> to
> pass as parameter value is not intuitive even if it shows the user that the
> parameter is nullable.
> 
> I therefore fear that there will be more NPEs if record is implemented in the
> same way as before.

yes,
Stephen Colebourne already reports that issue, the pattern with the constructor accepting null and the accessor returning an Optional is not supported.

You have two solutions:
- either you declare the component optional
  record Person(Optional<String> name) { }

- or don't declare the component Optional but burn the accessor
  record Person(String name /*may be null*/) {
    public String name() {
      throw new UnsupportedOperationException("name is optional");
    }
    public Optional<String> optionalName() {
      return Optional.ofNullable(name);
    }
  }
the problem with the second solution is that framework that uses reflection will use the accessor.

> 
> Inheritance
> 
> Less problematic I see the deep dependence of name in inheritance. I have a
> 
> interface Identity {
>    String id();
> }
> 
> The corresponding record looks something like this:
> 
> record ConcreteId(String id) implements Identity {
>    public ConcreteId {
>        this.id = requireNonNull(id);
>    }
> }
> 
> 
> Here I have a method name that has to become a parameter name so that I can use
> this parameter name again as instance variable name. Deep name coupling. I'm
> not sure if I like it yet.

A record component is not a parameter despite the record syntax using parenthesis,
it's a record component, something which is public and part of the public API, changing its name is not a backward compatible change.

Moreover, you can have less coupling if you want:
  interface Identity { String id(); }
  record ConcreteId(String value) implements Identity {
    public ConcreteId {
      this.value = requireNonNull(value);
    }
    public String id() { return value; }
  }

> 
> Have fun :-)
> 
> -isk

regards,
Rémi