Hi Tagir,
> On 21 Feb 2016, at 13:15, Tagir Valeev <[email protected]> wrote:
>
> Thank you for valueable coments, here's updated webrev:
>
> http://cr.openjdk.java.net/~tvaleev/webrev/8072727/r2/
>
> Changes:
> - Spec updated according to Brian Goetz suggestion
> - Predicate<? super T> wildcard
> - two-arg versions of iterate() are rewritten with Spliterator and started
> flag
> - IterateTest rewritten: now withData().stream().expectedResult().exercize()
> used; expected results added.
>
Looking good. Just minor comments.
For the primitive streams
—
+ Spliterator.OfDouble spliterator = new
Spliterators.AbstractDoubleSpliterator(Long.MAX_VALUE,
+ Spliterator.ORDERED | Spliterator.IMMUTABLE) {
+ double prev;
...
- return StreamSupport.doubleStream(Spliterators.spliteratorUnknownSize(
- iterator,
- Spliterator.ORDERED | Spliterator.IMMUTABLE |
Spliterator.NONNULL), false);
We should retain the NONNULL characteristic.
Thinking a little more that is something that should perhaps be added by
default by the primitive abstract spliterators. Something to consider later
maybe.
Stream
—
public static<T> Stream<T> iterate(final T seed, final UnaryOperator<T> f)
{
Objects.requireNonNull(f);
- final Iterator<T> iterator = new Iterator<T>() {
- @SuppressWarnings("unchecked")
- T t = (T) Streams.NONE;
I believe you can now remove the field Streams.NONE?
1186 public static<T> Stream<T> iterate(final T seed, final
UnaryOperator<T> f) {
1187 Objects.requireNonNull(f);
1188 Spliterator<T> spliterator = new
Spliterators.AbstractSpliterator<T>(Long.MAX_VALUE,
1189 Spliterator.ORDERED | Spliterator.IMMUTABLE) {
I forgot to mention before you can now use the “<>" on the RHS, although IDEs
might not like it javac will :-)
1270 @Override
1271 public void forEachRemaining(Consumer<? super T> action) {
1272 Objects.requireNonNull(action);
1273 if (finished)
1274 return;
1275 T t = started ? f.apply(prev) : seed;
1276 finished = true;
A really minor thing but if you move “finished = true” above one line, it means
there is one less place where unintended recursive traversal can occur.
IterateTest
—
57 {Arrays.asList(1, 2, 4, 8, 16, 32, 64, 128, 256, 512),
58 Factory.ofSupplier("ref.ten", () -> Stream.iterate(1, x ->
x < 1000, x -> x * 2))},
We now have the method “List.of", up to you if you wanna update to use that or
not.
Thanks,
Paul.
