JEP 269: Convenience Factory Methods for Collections

[Remi Forax]

Hi Kevin,

----- Mail original -----
> De: "Kevin Bourrillion" <kevinb at google.com>
> À: "Core Libs Dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Vendredi 25 Septembre 2015 17:33:02
> Objet: Re: JEP 269: Convenience Factory Methods for Collections
> 
> I don't think the proposal is bad, but for whatever it's worth to you... we
> wouldn't use it. And would probably advise all Guava users not to, either.
> In most cases new JDK functionality that replaces old Guava functionality
> comes out anywhere from "slightly" to "a lot" better, and in all these
> cases we advise switching to your stuff. But not this time, for at least
> one key reason.
> 
> Guava's immutable collections are very useful as *types*, not just as
> implementations. That is, while specific implementation classes are not
> exposed, abstract types like ImmutableList and ImmutableSortedSet are
> public (these should be thought of as "interfaces" in every logical sense
> except that they cannot be implemented outside the package).
> 
> When advising how to chose, say, a method return type, our advice is "use
> the most general type that still carries all the relevant semantic
> guarantees your caller should be able to depend on." The additional
> semantic information carried by our ImmutableSet type is that it (a) is a
> thread-safe immutable snapshot, (b) will never contain null, and (c)
> iterates in a deterministic order. Almost always, users do consider those
> semantic guarantees to be highly relevant. If the method could not return
> ImmutableSet, they would have to stuff a whole bunch of loose verbiage into
> their javadoc about it (which is effectively nearly equivalent to that
> information being lost).
> 
> Another example of the usefulness of types is that by making a field of
> type ImmutableMap, then if my constructor accepts a plain Map I *can't
> accidentally forget* to make a defensive copy. Another example is that if
> anyone does try to call .add(), we can issue an IDE warning.
> 
> We could have a debate over just *how* important this is (we've become
> solidly convinced over the years), but then, I'm not sure we see what the
> upside of using this (for anyone able to use Guava) would be anyway.

This proposal could be split in two parts, one is the introduction of static factory methods for common implementations, ArrayList, HashMap, etc,
the other is the introduction of immutable implementations for factory methods in interface.

I agree with you that having a way to express immutable collection types is big improvement for Java knowing that currently there is no way to enforce that a class is immutable, you just express that in the Javadoc. That said, i think that using an interface (or worst an abstract class) to represent an immutable collection type is not a good and flexible way to express those kind of types.
We have annotation on types since 8, having an annotation like @Immutable (with the IGJ checker framework by example) is IMO far better.
Unlike an interface, an annotation on type let you retrofit existing code by introducing the annotation without breaking existing code and reports as you said an error when you use a method that mutate the object.
The main drawback is that the annotation checker is something optional so you have to have your CI script correctly configured, not something that hard but still an issue.

> 
> (If, and somehow I feel this is unlikely, you were willing to add this
> fleet of immutable types as abstract classes, you could go with a more
> streamlined name than ours, like ImList. People would get used to it.)
> 
> Other quick comments:
> 
> Note that without permitting nulls, Map.of(key, Optional.of(value)) will
> become reasonably common, and that fact you can't serialize that will
> become even more strange than it already is.

Please don't do that, if you have a mapping between a key and something that doesn't exist, the best is to not have that mapping,
this is the semantics of java.util.concurrent (with null instead of Optional) and this is the only sane semantics.
Optional should never appear in collections/maps, it makes the code that deal with this kind of beast stupidly harder to read/write for no benefit.

> 
> I think the example of "double-brace initialization" should be even more
> clearly labeled as pure evil. :-) You could also mention all the horrible
> consequences if anyone ever serializes such a collection.

I fully agree, it also makes callsite megamorphic for no reason.

best,
Rémi

> 
> 
> 
> On Wed, Sep 23, 2015 at 5:02 PM, <mark.reinhold at oracle.com> wrote:
> 
> > New JEP Candidate: http://openjdk.java.net/jeps/269
> >
> > - Mark
> >
> 
> 
> 
> --
> Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb at google.com
>