RFR(m): 8139233u1 add initial compact immutable collection implementations
Stuart Marks
stuart.marks at oracle.com
Wed May 11 01:25:15 UTC 2016
On 5/9/16 7:25 AM, Remi Forax wrote:
> I still think that the code of hasNext of the iterator of SetN and MapN should not do any side effect.
Yes, I'm happy to consider further cleanups. I was starting to go off into the
weeds with refactoring and cleanups, since they were introducing regressions, so
I decided to check in a stable version and continue with cleanups later.
You had suggested to gather the searching code into a nextIndex() method, which
is called from the constructor and from next(), in order to simplify hasNext().
I see that HashMap's HashIterator is done this way, and in fact the search code
isn't in a method -- it's replicated between the constructor and nextNode(). See
HashMap.java lines 1477 and 1493. [1] [2]
My question is, does this really buy anything? In a typical
while(hasNext())/next() loop, the same work will get done, just in a different
place.
[1]
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/4da0f73ce03a/src/java.base/share/classes/java/util/HashMap.java#l1477
[2]
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/4da0f73ce03a/src/java.base/share/classes/java/util/HashMap.java#l1493
> otherwise,
> in the constructor of MapN, you can declare k and v as Object
> (the table is an array of Object and probe() now takes an Object)
Yes, good point.
> also instead of InternalError, i think AssertionError is a better exception,
> the javadoc of InternalError says that InternalError is thrown by the VM.
Hm. I agree that InternalError seems wrong since it's a subtype of
VirtualMachineError. But it's used about 2x more frequently than AssertionError
in the JDK.
> in MapN.probe, you can remove the cast to K (and the corresponding @SuppressWarnings).
Yes.
> Set2.e0 and Set2.e1 should be declared as package-private otherwise, the compiler generates an accessor in the iterator. Same thing for SetN.elements, MapN.table and MapN.size
I guess I privatized too many things. (But I'm no David Cameron.)
> CollSer implementation can be improved to avoid the ugly switch/case by replacing the constant list by an enum,
> something like the code below. In that case, creating a CollSer for a Map will be done like this,
> return new CollSer(CollSer.Kind.MAP, array);
I'll reply downthread after your exchange with Stephen Colebourne.
s'marks
More information about the core-libs-dev
mailing list