Fixes #426. Add fj.data.NonEmptyList.sequence*, traverse*, foldLeftN, foldRightN, init, last. #427
Conversation
…e*, foldLeftN, foldRightN, init, last.
| final F<A, B> transform) { | ||
| return tail().isEmpty() ? | ||
| transform.f(head()) : | ||
| init().foldRight(combine, transform.f(last())); |
There was a problem hiding this comment.
I think this is overly inefficient. init is O(n), last is O(n) and foldRight is O(n), so we have overall O(n^2). Compare this to List foldRight, which is O(n) for the reverse and then O(n) for foldLeft, for an overall O(n).
There was a problem hiding this comment.
I think we should be reversing and then folding left,
| /** | ||
| * Fold the list, from right to left. | ||
| * | ||
| * @param combine the combine function |
There was a problem hiding this comment.
For the param and return values, I don't think the textual description adds clarity to the types on the function parameters. Applies elsewhere.
| public final <B> B foldRightN( | ||
| final F2<A, B, B> combine, | ||
| final F<A, B> transform) { | ||
| return tail().isEmpty() ? |
There was a problem hiding this comment.
This and the F<A, F<B, B>> version should be implemented by one calling the other using curry/uncurry.
| final F<A, B> transform) { | ||
| return tail().isEmpty() ? | ||
| transform.f(head()) : | ||
| init().foldRight(combine, transform.f(last())); |
There was a problem hiding this comment.
I think we should be reversing and then folding left,
| return tail; | ||
| } | ||
|
|
||
| public List<A> init() |
There was a problem hiding this comment.
Trivial: Prefer the { on the same line as the method name. Applies elsewhere.
| * @param combine the combine function | ||
| * @return the output | ||
| */ | ||
| public final A foldRight1( |
There was a problem hiding this comment.
Prefer params on the same line as the method name unless it gets overly long, say >80 chars.
| public <R, B> Either<NonEmptyList<B>, R> traverseEitherLeft( | ||
| final F<A, Either<B, R>> f) { | ||
| return foldRightN( | ||
| element -> either -> f.f(element).left().bind(elementInner -> either.left().map(nonEmptyList -> nonEmptyList.cons(elementInner))), |
There was a problem hiding this comment.
This functions and those below use foldRightN, which needs fixing.
| public class NonEmptyListTest { | ||
| @Test | ||
| public void testSequenceEither() { | ||
| assertEquals(right(nel("zero")), sequenceEither(nel(right("zero")))); |
There was a problem hiding this comment.
I think using ints, instead of strings would be easier and better.
Fixes #426. Add fj.data.NonEmptyList.sequence*, traverse*, foldLeftN, foldRightN, init, last.