Add wrapper and instances for Sequence #241
Conversation
pt2121
changed the title
Codecov Report
@@ Coverage Diff @@
## master #241 +/- ##
=========================================
Coverage ? 49.76%
Complexity ? 240
=========================================
Files ? 113
Lines ? 2526
Branches ? 292
=========================================
Hits ? 1257
Misses ? 1154
Partials ? 115
Continue to review full report at Codecov.
|
|
looking good so far! |
pt2121
force-pushed
the
pt/sequence
branch
2 times, most recently
from
369a723
to
347ae0d
Compare
| } | ||
|
|
||
| testLaws(SemigroupKLaws.laws(SequenceKW.semigroupK(), applicative, eq)) | ||
| testLaws(MonoidKLaws.laws(SequenceKW.monoidK(), applicative, eq)) |
There was a problem hiding this comment.
MonoidKLaws already include SemigroupK laws :D
| init { | ||
| val eq: Eq<HK<SequenceKWHK, Int>> = object : Eq<HK<SequenceKWHK, Int>> { | ||
| override fun eqv(a: HK<SequenceKWHK, Int>, b: HK<SequenceKWHK, Int>): Boolean = | ||
| a.ev().sequence.toList() == b.ev().sequence.toList() |
There was a problem hiding this comment.
It'd be great to move .ev().sequence.toList() to an extension function, so it can be called directly like in here.
See: https://github.com/kategory/kategory/blob/master/kategory/src/main/kotlin/kategory/data/Id.kt#L3
|
this's ready for review |
|
|
||
| fun <A> empty(): SequenceKW<A> = emptySequence<A>().k() | ||
|
|
||
| private tailrec fun <A, B> go( |
There was a problem hiding this comment.
NIT: We generally want these functions as internal functions inside tailrecM
| fun <B> foldR(lb: Eval<B>, f: (A, Eval<B>) -> Eval<B>): Eval<B> { | ||
| fun loop(fa_p: SequenceKW<A>): Eval<B> = when { | ||
| fa_p.sequence.none() -> lb | ||
| else -> f(fa_p.ev().sequence.first(), Eval.defer { loop(fa_p.sequence.drop(1).k()) }) |
There was a problem hiding this comment.
one question also to @pakoito,
- Why not use
fa_p.ev().first()orfa_p.drop(1).k()instead ? - Why do you use
ev()method only forfa_p.ev().sequence.first()and not fordrop? - Is it necessary to use evidence method (
ev()) here?
There was a problem hiding this comment.
i think f(fa_p.first(), Eval.defer { loop(fa_p.drop(1).k()) }) works as well. i don't have a strong opinion on this. up to you guys
There was a problem hiding this comment.
Yes, it's works because you are delegating all public val sequence: Sequence<A> methods to SequenceKW
There was a problem hiding this comment.
Nits mostly, if @pt2121 wants I'll just go ahead and merge.
There was a problem hiding this comment.
i can update. gimme a couple minutes
pt2121
force-pushed
the
pt/sequence
branch
2 times, most recently
from
1d1d2e1
to
e74a526
Compare
make `go` an inner fun of `tailRecM` and improve foldR implementaion
|
done! |
|
@pt2121 Thank you for your effort. Happy to look something else in the roadmap for you! |
addressing #207