#68 Implement withFirst for primitive stream #174
Conversation
canthonyl
|
Thank you for your contribution! I don't like too much code duplication. Also code coverage decrease is too high. I suggest to put all the spliterators inside single class. Like |
| private static final int STATE_NONE = 0; | ||
| private static final int STATE_FIRST_READ = 1; | ||
| private static final int STATE_INIT = 2; | ||
| private static final int STATE_EMPTY = 3; |
There was a problem hiding this comment.
These constants can be shared between all implementations in abstract class
| private Spliterator.OfDouble source; | ||
| private DoubleWithFirstSpliterator prefix; | ||
| private volatile double first; | ||
| private volatile int state = STATE_NONE; |
There was a problem hiding this comment.
Fields lock and state could be shared as well
| } | ||
| } | ||
|
|
||
| private void release() { |
There was a problem hiding this comment.
Methods acquire and release could be shared
|
|
||
| private ReentrantLock lock; | ||
| private Spliterator.OfDouble source; | ||
| private DoubleWithFirstSpliterator prefix; |
There was a problem hiding this comment.
Prefix could probably be a shared field of abstract type
| if (prefixState != STATE_NONE) | ||
| return; | ||
| if (prefix != null) { | ||
| prefix.doInit(); |
There was a problem hiding this comment.
doInit() could be an abstract method of supertype, so prefix.doInit() could be called
| prefixState = prefix.state; | ||
| } | ||
| if (prefixState == STATE_FIRST_READ || prefixState == STATE_INIT) { | ||
| first = prefix.first; |
There was a problem hiding this comment.
For prefix.first probably it's ok to perform a type cast...
| private static final int STATE_EMPTY = 3; | ||
|
|
||
| private ReentrantLock lock; | ||
| private Spliterator.OfDouble source; |
There was a problem hiding this comment.
source could be a shared field with parameter type, so WithFirstSpliterator is parameterized with <T, SPLTR extends CloneableSpliterator<T>> extends CloneableSpliterator<SPLTR> and source field is declared as SPLTR source.
| } | ||
|
|
||
| @Override | ||
| public Spliterator.OfDouble trySplit() { |
There was a problem hiding this comment.
Ideally the whole trySplit() method implementation should be shared in parent abstract class.
| @@ -322,6 +322,11 @@ public DoubleStreamEx sorted() { | |||
| return new DoubleStreamEx(stream().sorted(), context); | |||
| } | |||
|
|
|||
| public DoubleStreamEx withFirst(DoubleBinaryOperator mapper) { | |||
There was a problem hiding this comment.
JavaDoc is missing for new API methods
| @@ -963,6 +963,42 @@ S doClone() { | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| static abstract class IntCloneableSpliterator<S extends IntCloneableSpliterator<?>> implements Spliterator.OfInt, | |||
There was a problem hiding this comment.
For me it seems that these three new classes are unnecessary. Looks like CloneableSpliterator can do the thing. No?
Interesting project! I've implemented 68 to add withFirst for primitive stream ex.
Please have a look. Unit Test case will follow.