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

Should factories like Semigroup.instance be provided for other typeclasses? #2887

Closed
morgen-peschke opened this issue Jun 12, 2019 · 2 comments · Fixed by #2889
Closed

Should factories like Semigroup.instance be provided for other typeclasses? #2887

morgen-peschke opened this issue Jun 12, 2019 · 2 comments · Fixed by #2889

Comments

@morgen-peschke
Copy link
Contributor

While Semigroup has cats.kernel.Semigroup#instance, Monoid has no equivalent, which could be implemented as:

object Monoid {
  /**
   * Create a `Monoid` instance from the given function and constant
   */
  @inline def instance[A](emptyVal: A, cmb: (A, A) => A): Monoid[A] = new Monoid[A] {
    override val empty: A = emptyVal
    override def combine(x: A, y: A): A = cmb(x,y)
}

Looking at the various typeclasses in cats.kernel, there doesn't seem to be a pattern for which ones have factories, and which don't:

Typeclass Has Factory Commentary
Band No Should be identical to Semigroup.instance
BoundedSemilattice No Should be identical to Monoid.instance
CommutativeGroup No See note for parent type Group
CommutativeMonoid No Should be identical to Monoid.instance
CommutativeSemigroup No Should be identical to Semigroup.instance
Eq Yes
Group No Group.instance(A, A => A, (A, A) => A) might be a bit much
Hash Yes
Monoid No Monoid.instance(A, (A, A) => A) should be easy to understand
Order Yes
PartialOrder Yes
Semigroup Yes
Semilattice No Should be identical to Semigroup.instance

Would a PR adding these be useful?

It doesn't look like factories for the typeclasses in cats.core would be possible, as they'd require accepting higher-order lambdas, which I don't think can be encoded as parameters in Scala.

@kailuowang
Copy link
Contributor

Thanks for identifying these potential factory methods. I think there are value in them. On the other hand, it raises a question in my head, will such methods gives user the false impression that implementing these instances is as simple as calling these methods? Maybe a more interesting question is how do we make it obvious to users that the instances must obey the laws and there are tests they can write to ensure their lawfulness? Admittedly it's not obvious in the current setup, but will these methods make it even less so?

@morgen-peschke
Copy link
Contributor Author

As cats-laws depends on cats-kernel, rather than the other way around, is it possible to include a link in the scaladoc to the laws it would need to conform to?

For example, would it compile to have a link to cats.laws.discipline.FunctorTests in the scaladoc for cats.Functor?

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 a pull request may close this issue.

2 participants