-
Notifications
You must be signed in to change notification settings - Fork 440
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 Foldable and Traversable instanced for most common classes #101
Conversation
Codecov Report
@@ Coverage Diff @@
## master #101 +/- ##
============================================
- Coverage 54.33% 52.58% -1.75%
- Complexity 146 164 +18
============================================
Files 67 76 +9
Lines 935 1025 +90
Branches 137 149 +12
============================================
+ Hits 508 539 +31
- Misses 372 429 +57
- Partials 55 57 +2
Continue to review full report at Codecov.
|
I've moved all ev() methods from the monads into the data classes. They were reused in typeclass instances other than monads. Better have them on a central spot. |
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.
good job @pakoito 👍 , only a few comments in order to understand better and styles.
fun <B> foldR(lb: Eval<B>, f: (A, Eval<B>) -> Eval<B>, FF: Foldable<F>, FG: Foldable<G>): Eval<B> = | ||
run.fold({ FF.foldR(it, lb, f) }, { FG.foldR(it, lb, f) }) | ||
|
||
fun <H, B> traverse(f: (A) -> HK<H, B>, GA: Applicative<H>, FT: Traverse<F>, GT: Traverse<G>) = |
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.
IMO It's better for a library POV always specify the returning type instead of infer it.
@@ -7,6 +7,8 @@ typealias CokleisiFun<F, A, B> = (HK<F, A>) -> B | |||
|
|||
typealias CoreaderT<F, A, B> = Cokleisli<F, A, B> | |||
|
|||
fun <F, A, B> CokleisiTKind<F, A, B>.ev(): Cokleisli<F, A, B> = this as Cokleisli<F, A, 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.
why moving it to top instead of leave it at bottom?
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.
For consistency. At the moment the ev() is available for all HK types. If it's at the bottom you have to make sure to add methods right before it, which it's a practice that may not be followed.
With this change, just from opening the file you know your alias and casts without having to scroll.
package katz | ||
|
||
/** | ||
* https://www.youtube.com/watch?v=wvSP5qYiz4Y |
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.
Really 😄
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 been know to comment worse :D
} | ||
} | ||
|
||
inline fun <F, reified G> Foldable<F>.compose(GT: Foldable<G> = foldable<G>()) = |
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.
just style could be in the same line
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.
also please specify the return type
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'm following the practice of breaking to the next line to separate signature and content. You can see the implementation at a glance even for short methods, rather than scrolling horizontally.
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 may have not been super consistent for shorter methods. I'll try to from now on.
inline fun <F, reified G> Foldable<F>.compose(GT: Foldable<G> = foldable<G>()) = | ||
ComposedFoldable(this, GT) | ||
|
||
data class ComposedTraverse<F, G>(val FT: Traverse<F>, val GT: Traverse<G>, val GA: Applicative<G>, val CF: ComposedFoldable<F, G> = ComposedFoldable(FT, GT)) |
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.
Please @pakoito could you explain why ComposeFoldable
is open but not a data class and ComposedTraverse
is not open but a data class?
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 did a refactor and forgot to fix it. Lemme go through it.
override fun <H, A, B> traverse(fa: HK<ComposedType<F, G>, A>, f: (A) -> HK<H, B>, HA: Applicative<H>): HK<H, HK<ComposedType<F, G>, B>> = | ||
HA.map(FT.traverse(fa.lower(), { ga -> GT.traverse(ga, f, HA) }, HA), { it.lift() }) | ||
|
||
fun <H, A, B> traverseC(fa: HK<F, HK<G, A>>, f: (A) -> HK<H, B>, HA: Applicative<H>) = |
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.
please specify the return type
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.
Specified!
} | ||
} | ||
|
||
inline fun <F, reified G> Traverse<F>.compose(GT: Traverse<G> = traverse<G>(), GA: Applicative<G> = applicative<G>()) = |
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.
please specify the return type
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.
Completed!
@@ -1,5 +1,9 @@ | |||
package katz | |||
|
|||
fun <A> (() -> A).k(): HK<Function0.F, A> = Function0(this) | |||
|
|||
fun <A> HK<Function0.F, A>.ev() = (this as Function0<A>).f |
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.
please specify the return type
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.
Done!
/** | ||
* https://www.youtube.com/watch?v=wvSP5qYiz4Y | ||
*/ | ||
interface ComposedType<out F, out G> |
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.
could it be private or break implicit magic?
My reason it's not expose it when you import kats that would confuse people that use 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.
It cannot be private, the compiler marks other instances that expose it like the constructor of the composed types. It also disallows lower(), which is used by the classes that use the composed constructs: https://github.com/FineCinnamon/Katz/blob/c4f790b9dff9201a82d338b9520860427e3289cf/katz/src/main/kotlin/katz/data/EitherT.kt#L72
This PR adds Foldable and Traversable instances to the following classes:
Validated
Coproduct
EitherT
Either
OptionT
Ior
Try
Id
There is some magic involved in OptionT and EitherT to simulate lambda types. This abstraction doesn't leak outside the package.