-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
The two implementations are effectively identical:
implicit def OrShow1[A: Show, B: Show]: Show[A Or B] =
Show.show[A Or B](_.fold(a => s"LOr(${Show[A].show(a)})", b => s"ROr(${Show[B].show(b)})"))
implicit def OrShow2[A, B](implicit showA: Show[A], showB: Show[B]): Show[A Or B] =
Show.show[A Or B](_.fold(a => s"LOr(${showA.show(a)})", b => s"ROr(${showB.show(b)})"))
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]
or ShowA: Show[A]
or A: 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?