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
