RFR(m): update 2: JEP 269 initial API and skeleton implementation (JDK-8139232)

[Stuart Marks]

On 12/4/15 1:50 AM, Stephen Colebourne wrote:
> Sorry for my delayed review. Basically, I'm happy with this.

Great!

> Map.ofEntries(Entry...)
> "The entry objects themselves are not stored in the map."
> This seems wrong. `Entry` might be implemented as a value, not an
> object in the future. And if so, it might form part of the state of an
> optimised list implementation.

I'll change this one to "The entries themselves...."

But we really want extract-and-copy semantics for the keys and values, 
explicitly leaving the entries behind. This is important if actual Map.Entry 
objects are passed in. Consider an instance of AbstractMap.SimpleEntry, which is 
mutable, or some other implementation of Map.Entry for which we can't make any 
guarantees.

It's also important for serializability. The return value of Map.entry() is not 
serializable, but we want the resulting Map to be serializable if possible. So 
we need to extract the keys and values.

In a value-typed future, if what's passed in really are values, then it's kind 
of nonsensical to talk about "the entries themselves" since they have no 
identity. But the requirement is necessary for objects.

> The use of <K,V> looks like it should have an extra space after the comma to me.

Old habit of mine. But the JDK code base is remarkably inconsistent about 
this... it seems like most generics code has no spaces if they are all 
one-letter type variables, but it has spaces if there are type bounds or 
(multi-letter) concrete type names. This is a pretty loose convention, though, 
if that.

In any case I've added spaces and I've fixed up the rest of the file.

> I prefer to avoid apostrophes in docs
> "the map's key type" -> "the key type of the map"
> "the first mapping's key" -> "the key of the first mapping"

Ugh. OK, I've indulged you with the other editorial changes, but this one goes 
too far. :-)

What's the rationale for this preference? Most cleanups suggested by you and 
others all make sense and conform to established conventions, but I don't think 
I've ever heard of this one before.

> I note that java.util.Stream uses empty() but this API uses of() for
> the empty case. I can see both points of view and don't have much of a
> preference. Is consistency with Stream needed?

Good point. I was aware of Stream.empty(), and I recall your early proposal had 
List/Set/Map.empty(). When I started on the first prototype, though, it seemed 
most natural to have the zero-arg of() be named consistently with the other 
overloads. That seems like it outweighs the inconsistency with 
Stream.of()/Stream.empty().

(I'll note that Stream.of() with zero args works, at the cost of creating an 
empty array.)

> KeyValueHolder needs a <p> at line 41

Fixed.

Thanks,

s'marks