Hi Mike,

Thanks for looking at this.

On Apr 16, 2013, at 5:39 AM, Mike Duigou <mike.dui...@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.

Reply via email to