search

typelevel / cats

Lightweight, modular, and extensible library for functional programming.
https://typelevel.org/cats/
Other
5.2k stars 1.19k forks source link

Make instance packages more consistent #2790

Open travisbrown opened 5 years ago

travisbrown commented 5 years ago

There's currently a lot of inconsistency in the way cats.instance and cats.kernel.instances are implemented, which can be extremely frustrating, especially for people trying to work with Cats from Java), or for contributors who want to know where to add something (e.g. me, in the past), or for anyone thinking of pointing to Cats as an example of a thoughtfully-structured Scala library (e.g. me, now).

For example, all cats.instances.xyz are defined as objects inside a cats.instances package object (which seems completely unnecessary and messy for Java users), except for cats.instances.symbol, which for some reason is defined as a package object in its own file. The cats-kernel instance packages follow the same approach, with each cats.kernel.instances.xyz being a package object.

Whether or not BinCompat and prioritizations traits are package private are also inconsistent across the instances packages.

In my view Cats should follow the cats-kernel approach consistently across both kernel and core, and all compat and prioritization traits should be made as private as possible without breaking the build. I'm happy to do this myself if people agree it should happen, but since these changes will break binary compatibility I guess they have to wait for 3.0? It's a shame to have to tell users who need Java compatibility that they have to roll a bunch of their own awful boilerplate because some bad decisions that made it into 1.0 will necessarily live 2+ years or whatever.

kailuowang commented 5 years ago

:+1: We recently just merged a PR to fix package objects in kernel https://github.com/typelevel/cats/pull/2785 I didn't know objects within package objects are pain for Java users. Under that light, I agree we should move them all to package objects. We might be about to make the change without breaking BC in the following way: Make current objects package private and deprecated. Create new replacements package objects. This passed the mima and compilation but I have tested it from user site yet. Better to test it in binCompatTest module. I can investigate more when I get back to computer later today. I also agree that all traits that aren't intended to be inherited externally should be made package private. That change should be BC. We should also add it to guidelines..

dwijnand commented 5 years ago

The best way I've seen to ensure good ergonomics in Java is by having Java tests (see akka/akka for examples of some duo-testing, maybe even some tidbits).

travisbrown commented 5 years ago

@dwijnand I'm not advocating putting a lot of effort into supporting usage from Java—this is more just avoiding gratuitously making life for Java users more difficult than it already is by doing stuff in Scala that isn't idiomatic anyway.

dwijnand commented 5 years ago

Even with the best intentions in mind, I've found the only way to safeguard is by actually writing the Java that uses the API. You get to (re)experience all those wonderful differences in syntax, in type inference, in user-site vs definition-site variance, in nesting, in access modifiers, etc... How much to invest in that, however, is up to the maintainers.