Skip to content
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

Typeclasses laws #112

Merged
merged 37 commits into from
Jun 24, 2017
Merged

Typeclasses laws #112

merged 37 commits into from
Jun 24, 2017

Conversation

raulraja
Copy link
Member

@raulraja raulraja commented Jun 15, 2017

  • FunctorLaws
  • ApplicativeLaws
  • MonadLaws
  • MonadErrorLaws (deferred to another PR since there is missing instances)

@raulraja raulraja requested review from pakoito and ffgiraldez June 15, 2017 21:44
@pakoito pakoito changed the title WIP typeclasses laws Typeclasses laws Jun 15, 2017
@@ -1,6 +1,6 @@
package katz

class EitherMonad<L> : Monad<EitherF<L>> {
open class EitherMonad<L> : Monad<EitherF<L>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this class need to be open? Could it be an Interface instead?

@@ -3,14 +3,14 @@ package katz
typealias KleisiTKind<F, A, B> = HK3<Kleisli.F, F, A, B>
typealias KleisiF<F> = HK<Kleisli.F, F>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe these two are misnamed too. Could you please use this diff to rename them to Kleisli?

Copy link
Member

@ffgiraldez ffgiraldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some non blockers questions


object FunctorLaws {

inline fun <reified F> laws(AP: Applicative<F> = applicative<F>()): List<Law> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why use an Applicative here, if we are testing Functor laws here? should it be convenient use Functor instead of Applicative ins't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't create instances in the generator unless we create a specific typeclass for pure or apply since that is defined in Applicative and that is what we need to lift values into F[_]

M.flatMap(fa, { M.pure(it) }) == fa
})

inline fun <reified F> kleisliLeftIdentity(M: Monad<F> = monad<F>()): Unit =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question,

why in MonadLaws have the particular case for Kleisli Monad and not split into MonadLaws and KleisliLaws that have all the MonadLaws plus those 2 specific test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Kleisli applies to all monads.
https://en.wikipedia.org/wiki/Kleisli_category

In category theory, a Kleisli category is a category naturally associated to any monad T. It is equivalent to the category of free T-algebras.

@raulraja
Copy link
Member Author

@kategory/maintainers I suggest we merge this now since master is already broken in many ways and this proofs some of the types that need immediate action

Copy link
Member

@pakoito pakoito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll happily pass it, will work on the problems ASAP. I believe it may have to be rebased on master first, I'm not a fan of squashing.

@raulraja raulraja merged commit 6bfa149 into master Jun 24, 2017
@raulraja raulraja deleted the rr-discipline branch June 24, 2017 13:24
@pakoito pakoito mentioned this pull request Jul 16, 2017
@wiyarmir wiyarmir mentioned this pull request Jul 17, 2017
ambrusadrianz pushed a commit to ambrusadrianz/arrow that referenced this pull request Oct 22, 2019
rachelcarmena pushed a commit that referenced this pull request Feb 24, 2021
* Suspend fx

* Code review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants