Add Semiring type class

[juliankotrba]

• Interface definition
• Laws
• Instances
• Tests
• Documentation

closes #1225

[juliankotrba]

Hey @pakoito, here are some things I would like to know for this PR:

• Should I create (internal?) classes for multiplicative monoid/semigroup and let Semiring extend them
• Any better idea for naming the function product
• Did I forget something else important to add?

Thank you for any help you can provide 🍻

[pakoito]

Awesomely done, thank you much for putting time on this.

Should I create (internal?) classes for multiplicative monoid/semigroup and let Semiring extend them

I haven't seen any docs, talks, or examples where each was useful individually. Maybe with KEEP-87 when we enforce unique instances? I wouldn't require them for this PR tho, this is enough!

Any better idea for naming the function product

I checked on other languages and they tend to use either times or an operator. Because we like verbose probably combineMultiplicate is the best choice here.

Did I forget something else important to add?

Nothing important, just some short KDocs atop of the typeclass and the functions would be nice. I'm bad at this myself so it isn't a requirement :)

[pakoito]

The build failed on a flaky tests, so don't worry about it!

[raulraja]

@juliankotrba great stuff, Take a look at the Functor type class sources for examples on how to document a type class.

[juliankotrba]

Thank you both @pakoito @raulraja for your feedback! I think I did also something wrong with the number instance tests. I am currently calling equalUnderTheLaw in my Semiring laws. But equalUnderTheLaw is just returning true or false and not throwing an AssertionException, hence my tests will never fail if there is something wrong with the implementation.

For me it looks like that shouldBeEqual is designed for this use case but is currently just creating a io.kotlintest.Result. Shouldn't it be something like this:

if (eqv(b).not()) {
      throw AssertionError("Expected: $this but found: $b")
}

Please correct me if I am wrong or miss something.

[pakoito]

equalUnderTheLaw is meant to be used with property-based tasting blocks forAll. If you just want to test a single case, useshouldBe for assertions.

[pakoito]

Could it be fun A?.maybeCombineMultiplicate(b: A?): A = instead?

[juliankotrba]

Sure, but I wanted to be consistent with Semigroup's maybeCombine

[juliankotrba]

Should I change it in this this PR for both functions, create a new PR or just leave it?

[pakoito]

I'd fix both on this PR tbh :P

[juliankotrba]

K 😁 But just to be sure, returning zero() if the receiver is null is the expected behavior, right?

[pakoito]

zero for addition, one for multiplication

[juliankotrba]

Mhh maybe the reason why maybeCombine's receiver is not nullable is because maybeCombine is defined in Semigroup, which doesn't have an empty element. To get around this I have added A?.maybeCombineAddition to Semiring

[pakoito]

@juliankotrba Great work! tell us when it's ready to go!

[raulraja]

Once you add the Kdocs you can remove this file and have the menu entry point to the generated apidocs. That already includes the type class hierarchy charts and will turn any kotln:ank:playground docs in runnable snippets in the arrow docs. The Kdocs are written directly in the class. See functor for an example. We no longer create files for each type class or data type and the ones we have are in the process of being converted to just point to the apidocs.

[juliankotrba]

I tried to update everything as you suggested, but I'm still doing something wrong. Ank is able to compile everything without errors but when running the Ank playground examples, it fails because of unresolved references to Int.semiring(). I guess it is basically the same problem as here in the playground example. If I change semiring with semigroup everything runs without problems. I also checked if the classpath of the docs module contains an up to date arrow-core-extensions.jar, because the Int.semiring() extension function is declared inside.
Any ideas?

[juliankotrba]

@raulraja I just figured out that Ank has nothing to do with the actual playground snippet (:man_facepalming:), so I guess the problem is that the arrow playground server doesn't know about my changes yet. Sorry, my bad..

[pakoito]

Stamping the PR to merge as soon as it's ready!

[raulraja]

@juliankotrba Outstanding work and thank you so much for this contribution. 👏

[juliankotrba]

Thank you guys for the great support !!