-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement more flows ops methods part 3 #76
Conversation
68b814c
to
03d328c
Compare
* groupBy * groupedWithin * groupedWeightedWithin * mapStateful * mapStatefulConcat + alsoTo small fix
03d328c
to
2498438
Compare
* @param element current input flow element | ||
* @return pair of new state and `element` mapped to new type `U` | ||
*/ | ||
Map.Entry<S, U> apply(S state, T element); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can stick with Map.Entry or create custom Pair implementation (I believe there is no point in depending on other library for this).
The main problem with Map.Entry is that it does not allow null as first
/key
argument (which makes sense from Map implementation point of view), but it created some problems for next implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Map.Entry sounds good - do we really use null
anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In ox we used null as marker for lines
method.
I used Optional for workaround https://github.com/softwaremill/jox/pull/77/files#diff-46d2537d747d24775cf70630d584ac42326ea6902d3ddd10f5a861841c2c3b6cR722
Overall we can leave it as it is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, Optional is better :) 👍
* @param <U> type of output flow | ||
*/ | ||
@FunctionalInterface | ||
public interface StatefulMapper<T, S, U> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I've added those interfaces to make the code more readable. If it makes it more blurry for you, I can remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this looks good :)
import com.softwaremill.jox.structured.Scopes; | ||
import com.softwaremill.jox.structured.UnsupervisedScope; | ||
|
||
class GroupByImpl<T, V, U> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewritten from Scala
import java.util.NoSuchElementException; | ||
import java.util.Optional; | ||
|
||
class WeightedHeap<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewritten from scala
import com.softwaremill.jox.flows.WeightedHeap.HeapNode; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class WeightedHeapTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All rewritten from Scala
Closes #68