RFR JDK-8011426: java.util collection Spliterator implementations

Paul Sandoz paul.sandoz at oracle.com
Tue Apr 16 04:51:27 PDT 2013


Hi Mike,

Thanks for looking at this.

On Apr 16, 2013, at 5:39 AM, Mike Duigou <mike.duigou at oracle.com> wrote:

> I went back and started again with the 8010096 webrev.
> 
> Spliterators::
> 
> - some implementations are private and some are package private. All package private?
> 

I don't think there is any need to make EmptySpliterator package private. 

All other static spliterator classes are package private, although not all of them are used by other classes in the same package. Arguably the right thing to do make such classes package private only if they are required to be so.


> List/Set/Iterator/SortedSet::
> 
> - Include the same interface level @implSpec warning as Collection?
> 

I don't think it applies to Iterator (or Iterable). For unmodifiable collections the caller is required to explicitly synchronize iteration since one could do:

  i.next()
  i.forEachRemaining(...)

I would be inclined to leave it as is rather than repeat on all interfaces and implementations that can extended.


> Spliterator::
> 
> - "<p>Spliterators also report ..." You may want to avoid the plural form since there's also a class of that name.
> 

OK, i changed this one. The plural is used in many other places too, but the context is clear enough for those i think.


> Spliterator.NONNULL - "This applies, for example, to ...". I might like the name Spliterator.NONULLS better.
> 
> Spliterator.IMMUTABLE - this name doesn't quite capture what's allowed and what's prohibited.

It is about structural modification to the source:

     * Characteristic value signifying that the element source cannot be
     * structurally modified; that is, elements cannot be added, replaced, or
     * removed, so such changes cannot occur during traversal. A Spliterator


> An Arrays.asList() list is IMMUTABLE but elements can still be replaced in place. Collections.unmodifiableList(Array.asList(..)) is entirely immutable. Is the distinction ever important?
> 

Ah, well spotted, that is a bug in the implementation of Arrays.ArrayList implementation, it should not be IMMUTABLE, since we anyway cannot guarantee immutability of the backed array. I removed the declaration of IMMUTABLE.


> - I guess the issue is that some of the flags describe characteristics of the source and some describe characteristics of the elements.
> 

Yes, in this case IMMUTABLE really is about the source, since the elements themselves could be modified (just like for unmodifiable collections).


> - "A diagnostic warning that boxing of primitives values is occurring of can be requested by setting the boolean system property {@code org.openjdk.java.util.stream.tripwire} is to {@code true}."
> 

That is a little vague because it does not say how they occur. I changed to:

 * @implNote
 * If the boolean system property {@code org.openjdk.java.util.stream.tripwire}
 * is set to {@code true} then diagnostic warnings are reported if boxing of
 * primitive values occur when operating on primitive subtype specializations.


> - Neither forEachREmaining on Iterator or tryAdvance on Spliterator say whether it's possible (or advisable) to continue advancing following an exception being thrown. Will calling again continue with the next element? The same element? Unspecified? Is calling again after an exception prohibited?
> 

Undefined behaviour, a bit like if any unspecified runtime exception or error is thrown by the Iterator/Spliterator itself. Exceptions should not be used for flow control with forEachRemaining.

We could say for Iterators:

  An error or runtime exception thrown by the action is relayed to the caller, and the results of further iteration are undefined.

and for Spliterators:

  An error or runtime exception thrown by the action is relayed to the caller, and the results of further traversal or splitting are undefined.

?


> - getExactSizeIfKnown() - use hasCharacteristics?
> 

We could, it is marginally more efficient not to.


> - The note in CONCURRENT "Such a Spliterator is
>     * inconsistent and no guarantees can be made about any computation using
>     * that Spliterator." Is this necessary or just confusing. Users won't encounter this.
> 
> - Same with the similar note in SUBSIZED. 
> 

OK, Brian removed that.

Paul.


More information about the lambda-dev mailing list