Add Semigroupal type class #1280
Conversation
juliankotrba
changed the title
modules/core/arrow-typeclasses/src/main/kotlin/arrow/typeclasses/Semigroupal.kt
Outdated
Show resolved
Hide resolved
Open one so we can keep track, yeah. |
modules/core/arrow-core-extensions/src/main/kotlin/arrow/core/extensions/option.kt
Show resolved
Hide resolved
modules/core/arrow-extras/src/main/kotlin/arrow/data/SequenceK.kt
Outdated
Show resolved
Hide resolved
juliankotrba
changed the title
|
@juliankotrba ready to review? |
Yes, please. |
modules/core/arrow-test/src/main/kotlin/arrow/test/laws/SemigroupalLaws.kt
Outdated
Show resolved
Hide resolved
| ) | ||
|
|
||
| private fun <F, A, B, C> Semigroupal<F>.semigroupalAssociative(af: Kind<F, A>, bf: Kind<F, B>, cf: Kind<F, C>, bijection: (Kind<F, Tuple2<Tuple2<A, B>, C>>) -> (Kind<F, Tuple2<A, Tuple2<B, C>>>), EQ: Eq<Kind<F, Tuple2<A, Tuple2<B, C>>>>): Unit = | ||
| af.product(bf.product(cf)).shouldBeEq(bijection(af.product(bf).product(cf)), EQ) |
There was a problem hiding this comment.
We should prefer forAll and eqUnderTheLaw rather than this.
modules/core/arrow-typeclasses/src/main/kotlin/arrow/typeclasses/Semigroupal.kt
Outdated
Show resolved
Hide resolved
|
Thanks for the review @pakoito, I will try to fix everything until next week 😊 |
|
Master is broken and it's my fault. I'll fix it tonight |
|
ready to merge when you think it is! |
|
It's silly but there's one detekt error: |
| this.should { | ||
| io.kotlintest.Result(eqv(b), "Expected: $this but found: $b", "$this and $b should be equal") | ||
| } | ||
| this.should(object: Matcher<Eq<A>> { |
There was a problem hiding this comment.
Imho the method now does what it was originally intended for. We could discuss it with the author and then put it in its own PR, make the change here or revert it.
There was a problem hiding this comment.
The value in override fun test(value: Eq<A>): Result { is never used, so there's something there that doesn't add up.
There was a problem hiding this comment.
yeah right, thats not that clean. Additional changes should then be made in new PR. I will revert for now and keep this thing in mind.
|
Thank you @juliankotrba for all the good work, and thanks for being receptive of the code review :D |
closes #882 #951