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

Imports unification and other syntax changes #117

Open
FunFunFine opened this issue Nov 28, 2021 · 3 comments
Open

Imports unification and other syntax changes #117

FunFunFine opened this issue Nov 28, 2021 · 3 comments

Comments

@FunFunFine
Copy link

FunFunFine commented Nov 28, 2021

There is a little inconsistency regarding imports in the library: while any other library (e.g. cats, cats-effect & tofu) import their syntax by import lib.syntax.something._ and import lib.syntax.all._, cats-helper does it in its own unique way by import com.evolutiongaming.SomethingHelper._.

It kind of slows down the adoption by newcomers, because they can easily miss that those syntax objects even exist, so I would like to propose to change this import scheme to be the same as in cats. WDYT?

Another thing I would like to discuss here (I don't think it is worth creating a separate issue for that) is the pollution by apply methods. One can easily find such code in cats-helper (e.g. Blocking, FromFuture):

trait Something[F[_]] {
  def apply(something: Thing): F[OtherThing]
}

object Something {
  def apply[F[_]: Something]: Something[F] = implicitly
}

Not only that it just doesn't work to call it like Something[F](thing) because of double apply and thing being treated as an explicitly passed instance, but IntelliJ IDEA often fails to find usages or asks if I want to go to the instance or to the method. In other words, it is inconvenient. So my proposal is to add alternative methods names with the possible deprecation of apply methods.

I could get this done, but I would like to see some discussion here. Are those proposals any good?

@t3hnar
Copy link
Contributor

t3hnar commented Nov 29, 2021

@FunFunFine not sure this does really makes a lot of sense, as comparing to cats/cats-effect - we don't have much to import hence not sure feature of being able to "selectively import something" is useful

As drawback - there is added maintenance and runtime cost of flexible solution

@FunFunFine
Copy link
Author

@FunFunFine not sure this does really makes a lot of sense, as comparing to cats/cats-effect - we don't have much to import hence not sure feature of being able to "selectively import something" is useful

As drawback - there is added maintenance and runtime cost of flexible solution

My proposal is more about syntax.all._ part. Currently there are seven different *Helper objects which need to be imported separately. Maybe we should allow them all to be imported with just one object then (e.g. import com.evolutiongaming.catshelper.syntax._)?

@mr-git
Copy link
Contributor

mr-git commented Dec 1, 2021

With upgrade to CE3 and changing package to com.evolution, this would be very welcome change (if one is afraid to add such nicer import in point release).

I've had numerous times, when I had to lookup which is the required import, thus having something common, very similar to Cats and Cats-Effect approach, would be very welcome usability improvement.

Nobody is going to remove the ability to have explicit single given imports.

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

No branches or pull requests

3 participants