[11] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

Stuart Marks stuart.marks at oracle.com
Thu Mar 22 00:51:43 UTC 2018


Hi Claes,

I'm finally finally getting back to this. I reviewed the current state of the 
patch located here:

     http://cr.openjdk.java.net/~redestad/8193128/open.06/

I think this is mostly fine as it is. There are some things about it that could 
be adjusted, but none of them stand in the way of it going in. If you want to 
take care of any of these items before pushing, that'd be great, otherwise they 
can be handled separately later.

* AbstractImmutableCollection and AbstractImmutableSet are in the "List 
Implementations" section of the file. Seems like AIC ought to be moved to the 
top, since it's common to List and Set, and AIS ought to be moved to the "Set 
Implementations" section. This is just location in the file, not nesting level.

* The SubList constructors that are overloaded based on the type of the 1st arg 
(List vs SubList) seems subtle and error-prone. I misread the code the first 
time I saw it. Seems like it would be preferable to have well-named static 
factory methods, each calling a policy-free (private) constructor.

* Should SubList.size be final? Should any of the SubList fields be @Stable?

* Does the SubList class need to nested within AbstractImmutableList? Note that 
it also extends AIL, so I don't think nesting gives it access to anything that 
it doesn't already have. Plus it includes an anonymous inner class based on 
ListIterator, which is a fourth level of nested classes. It'd be good to flatten 
this out a bit if possible.

* The instance returned by SubList.iterator() is also a ListIterator. Hmmm. But 
I'm also wondering if SubList's iterators can be shared somehow with AIL's Itr 
and ListItr.

* Several indexOf tests are added to ListFactories.java. Are these redundant 
with the indexOf tests that were added to MOAT?

Let me know what, if any, you fix up before pushing, and I'll track the rest.

Thanks,

s'marks


More information about the core-libs-dev mailing list