-
Notifications
You must be signed in to change notification settings - Fork 450
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 Alternative, Bifoldable, and MonadCombine #261
Conversation
5b93869
to
ef48219
Compare
Codecov Report
@@ Coverage Diff @@
## master #261 +/- ##
========================================
Coverage ? 45.3%
Complexity ? 319
========================================
Files ? 149
Lines ? 3803
Branches ? 423
========================================
Hits ? 1723
Misses ? 1946
Partials ? 134
Continue to review full report at Codecov.
|
ef48219
to
0a69845
Compare
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.
Outstanding work 👏 👏 awesome to see more typeclasses we initially thought impossible coming into Kategory 🎉 . Let's update this branch with master's new style. There are new tests to make sure the declared implicit typeclasses work implcitly that you would have to add to all the new instances once you get latest from master.
@@ -1,7 +1,7 @@ | |||
package kategory | |||
|
|||
@higherkind | |||
@deriving(Monad::class, Traverse::class, MonoidK::class) | |||
@deriving(Monad::class, Traverse::class, MonoidK::class, MonadCombine::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.
To be consistent with the rest of typeclasses in and derived instances we should here list all of the instances individually... Functor, Applicative, Monad, SemigroupK, MonoidK,...
@@ -5,7 +5,8 @@ typealias StateTFunKind<F, S, A> = HK<F, StateTFun<F, S, A>> | |||
|
|||
fun <F, S, A> StateTKind<F, S, A>.runM(initial: S): HK<F, Tuple2<S, A>> = (this as StateT<F, S, A>).run(initial) | |||
|
|||
@higherkind class StateT<F, S, A>( | |||
@higherkind | |||
class StateT<F, S, A>( |
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.
The ones currently in master merged in #264 already include the refactoring for StateT that is consistent in naming conventions and implementations with the rest of the code base. You can accept master
changes when updating this branch.
@@ -1,25 +1,28 @@ | |||
package kategory | |||
|
|||
import kategory.Option.None | |||
import kategory.Option.Some | |||
|
|||
interface OptionMonoid<A> : Monoid<Option<A>> { |
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 as mentioned for StateT
the ones in master have been already addressed
} | ||
is Option.None -> a | ||
is None -> a | ||
} | ||
|
||
} | ||
|
||
data class OptionMonadError<E>(val error: E) : MonadError<OptionHK, E> , OptionMonadInstance { |
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 as mentioned for StateT
the ones in master have been already addressed
fun F(): Monad<F> | ||
fun G(): SemigroupK<F> | ||
|
||
override fun <A> combineK(x: HK<HK2<StateTHK, F, S>, A>, y: HK<HK2<StateTHK, F, S>, A>): StateT<F, S, A> = | ||
StateT(F(), F().pure({ s -> G().combineK(x.ev().run(s), y.ev().run(s)) })) | ||
} | ||
|
||
interface StateTMonadCombine<F, S> : MonadCombine<StateTKindPartial<F, S>>, StateTMonad<F, S>, StateTSemigroupK<F, S> { |
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.
This instance requires an implicit object as introduced by #264
@@ -0,0 +1,16 @@ | |||
package kategory | |||
|
|||
interface Bifoldable<in F> : Typeclass { |
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.
Contravariant in
positions don;t work currently in the implicits system because java reflection does not see those how we need them to lookup instances by type so we should remove in
positions from typeclasses.
@@ -0,0 +1,16 @@ | |||
package kategory | |||
|
|||
interface Bifoldable<in F> : Typeclass { |
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.
Most datatypes that currently support Bimonad
and Foldable
are probably gonna be able to implement this typeclass.
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.
Which ones ?
|
||
import io.kotlintest.properties.forAll | ||
|
||
object BifoldableLaws { |
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.
👏
0a69845
to
525efc0
Compare
Everything is rebased / fixed / following your typeclass instances derivation system. I think I didn't miss anything so please, take another look when you have a moment @raulraja. |
Excellent! |
Fixes #257 and #248 (very related)
AlternativeLaws
MonadCombine
MonadCombineLaws
Bifoldable
BifoldableLaws
MonadCombineLaws
andAlternativeLaws
using a stateT monadCombine instance.BifoldableLaws
using an instance ofComposedBifoldable
.StateT
instances to avoid asking for not needed dependencies on each one of them. This also allowed to add two new ones:StateTMonadCombine
andStateTMonadError
.