Skip to content

Add Monoidal type class#1327

Merged
pakoito merged 8 commits intoarrow-kt:masterfrom
juliankotrba:feature/monoidal
Mar 12, 2019
Merged

Add Monoidal type class#1327
pakoito merged 8 commits intoarrow-kt:masterfrom
juliankotrba:feature/monoidal

Conversation

@juliankotrba
Copy link
Contributor

@juliankotrba juliankotrba commented Mar 3, 2019

  • Interface definition
  • Laws
  • Instances
    • Option
    • ListK
    • SequenceK
    • SetK
  • Tests
  • Documentation

closes #1323

@pakoito
Copy link
Member

pakoito commented Mar 10, 2019

Ready to review whenever you're ready to publish :D

@juliankotrba
Copy link
Contributor Author

Ready to review but I have no idea how to change the PR to Ready to review 😅. Maybe that's because a check hasn't finished yet?

@pakoito
Copy link
Member

pakoito commented Mar 11, 2019

I can see it here.

Screen Shot 2019-03-11 at 11 14 29 AM

I'll click the button!

@pakoito pakoito marked this pull request as ready for review March 11, 2019 11:15
@pakoito
Copy link
Member

pakoito commented Mar 11, 2019

Branch has conflicts, sadly


private fun <F> Monoidal<F>.monoidalLeftIdentity(f: (Int) -> Kind<F, Int>, EQ: Eq<Kind<F, Tuple2<Int, Int>>>): Unit =
forAll(Gen.int().map(f)) { fa: Kind<F, Int> ->
identity<Int>().product(fa).equalUnderTheLaw(identity(), EQ)
Copy link
Member

@pakoito pakoito Mar 11, 2019

Choose a reason for hiding this comment

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

I thought this would be equal to fa and not identity!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course you are right, don't know why I implemented it like this. Thanks for pointing that out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I remember why I did it like this. For example, if we look at ListK:
fa = {1, 2}
fb = {}
The current outcome of fa x fb is {}
{} is also the identity element of ListK, so the outcome within a Monoidal should actually be fa x fb = fa and not fa x fb = {}. This kinda confuses me 😅. It would be great if you could shed some light on this.

Copy link
Member

Choose a reason for hiding this comment

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

The current outcome of fa x fb is {}

It shouldn't, should it? AFAIK the outcome should be the same as with monoid. That is, the concatenation of both lists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am understanding the Semigroupal type class as an abstraction over the cartesian product so the current behaviour is fa x {} = {}

Copy link
Member

Choose a reason for hiding this comment

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

Ooooooh. Never mind me then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then I still have to change the documentation, right?

*
* [identity] returns a specific identity `Kind<F, A>` value for a given type [F] and [A].
*
* Like any other Monoid type class, [Monoidal] also complies with the left and right identity law:
Copy link
Member

Choose a reason for hiding this comment

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

Same as in the laws, I thought that the product of empty and a is a, and not empty

@juliankotrba
Copy link
Contributor Author

I can see it here.

Screen Shot 2019-03-11 at 11 14 29 AM

I'll click the button!

The button is no longer visible to me .

@pakoito pakoito merged commit 3218a08 into arrow-kt:master Mar 12, 2019
@pakoito
Copy link
Member

pakoito commented Mar 12, 2019

And in! Thank you again @juliankotrba for working on this :D Hope you had fun

@raulraja raulraja mentioned this pull request Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Monoidal type class

2 participants