-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Typeclass instance usage convention #27
Comments
I agree with pretty much all of this: I think we should prefer As far as the names of the evidence parameter, I know @tixxit prefers |
(And yes, we do need a contributor guide that spells this out.) |
I always liked the idea to view the |
Implements feedback from typelevel#11. Specifically: 1. LOr and ROr move to `Or.LeftOr` and `Or.RightOr`. 2. Constructors are still public, but `left` and `right` preserved in `OrFunctions` to infer `Or`. Use what you like. 3. Only the Scaladoc is ripped from Scalaz. Also tries to adhere to the emerging consensus on typelevel#27. Preliminar benchmarking showed monomorphism is faster than polymorphism, and that implementing other operations using fold/bimap is immeasurable. The second finding is dubious with respect to allocations. If someone can prove these guilty via benchmark, we can revisit. I'd like to add tests after typelevel#29 happens.
shall we close this ticket? |
Yes, I think we have reached broad agreement. |
The two implementations are effectively identical:
In order to make it easier for newcomers to contribute, I think cats should establish and document a convention for which of the two approaches to use.
Here is a proposed convention:
If your function needs to call a method on the type class instance, it should use the latter approach. This prevents the unnecessary calls to
Show.apply
.If your function just needs the evidence of the type class instances so it can call through to another function that requires them, it should use the former aproach, as it is a bit cleaner.
There are also questions about whether you should use names like
showA: Show[A]
orShowA: Show[A]
orA: Show[A]
, etc. I don't really have a preference on convention here, but I think the more of this we can put into a contributor guide the less people have to worry about not knowing the right way to do it or submitting a pull request that gets nitpicked.Thoughts?
The text was updated successfully, but these errors were encountered: