-
Notifications
You must be signed in to change notification settings - Fork 448
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
Add laws for Foldable instances #128
Conversation
Codecov Report
@@ Coverage Diff @@
## master #128 +/- ##
=========================================
Coverage ? 55.65%
Complexity ? 211
=========================================
Files ? 90
Lines ? 1335
Branches ? 174
=========================================
Hits ? 743
Misses ? 518
Partials ? 74
Continue to review full report at Codecov.
|
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.
Excellent!
fun <G, A, B> foldM(MG: Monad<G>, fa: HK<F, A>, z: B, f: (B, A) -> HK<G, B>): HK<G, B> { | ||
return foldL(fa, MG.pure(z), { gb, a -> MG.flatMap(gb) { f(it, a) } }) | ||
} | ||
fun <G, A, B> foldM(MG: Monad<G>, fa: HK<F, A>, z: B, f: (B, A) -> HK<G, B>): HK<G, B> = |
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.
should this contain the default param value = monad()
?
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.
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.
what if it is expressed as an extension function?
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.
That might work. I'm starting to seriously consider that we'd remove several headaches if we were to express all functions bar the pure virtual ones as extensions.
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.
I've added them to this PR #129
fun <G, A, B> foldMapM(MG: Monad<G>, bb: Monoid<B>, fa: HK<F, A>, f: (A) -> HK<G, B>) : HK<G, B> { | ||
return foldM(MG, fa, bb.empty(), { b, a -> MG.map(f(a)) { bb.combine(b, it) } }) | ||
} | ||
fun <G, A, B> foldMapM(MG: Monad<G>, bb: Monoid<B>, fa: HK<F, A>, f: (A) -> HK<G, B>) : HK<G, B> = |
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.
Same for monoid
Working towards #103
In this PR the test for all Foldable laws are added. I'll add the last for Traverse on the next PR, so we can have both reviewed separately.
Reference: https://github.com/typelevel/cats/blob/master/laws/src/main/scala/cats/laws/TraverseLaws.scala
I have tested them with Id and Coproduct, which will get Traverse laws on the next PR instead.