typelevel / cats

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

[PROPOSAL] Aliases for methods `traverse_` and `sequence_` #4611

Open satorg opened 1 month ago

satorg commented 1 month ago

Since fa.traverse_(fun) and fa.sequence_ are semantically the same as fa.traverse(fun).void and fa.sequence.void I would propose to add aliases traverseVoid and sequenceVoid for them correspondingly.

Why?

In two words: discoverability and clarity.

The names traverse_ and sequence_ look cryptic and not self-explaining at all. I personally witnessed guys who were relative new to Cats and they were able to figure out regular traverse and sequence theirselves but were struggling with the traverse_ and sequence_ variants. Most likely this is because across the industry underscores in method names are often used to mark some kind of internal/system API which should not be normally in use.

djspiewak commented 1 month ago

Normally I hate having multiple words for things but… yeah, the underscore variants are really weird, particularly for something so commonly used. Agreed this is worth fixing. Also we've done this in a few places in Cats Effect as well, perhaps most notably async_ (though in our defense on that one, it is the variant you probably shouldn't be using).

danicheg commented 1 month ago

As of my two cents, technically, that underscore postfix has been well-recognized and has influenced the whole ecosystem (not only the Typelevel one). We shouldn't give up on it, for sure. But it's also true that adding a few more aliases will barely make things worse. To consider:

johnynek commented 1 month ago

Personally, I think traverseVoid is better than traverse_. It is a contraction of traverse(_).void and it hopefully helps the user also learn about .void.

While traverseUnit seems plausible, note that def unit: F[Unit] which is a constant, while def void[A](f: F[A]): F[Unit] is not.. so traverseUnit invites that confusion in my opinion.

Discard is also possibly nice, but if we go down that road, I think we want def discard[A](f: F[A]): F[Unit] added to functor as a synonym for void. Secondly, the "discard" part is just the inside value, whereas if you read discard(List(1, 2, 3)) it's not obvious that is just discarding the inside, not the entire thing (not sure why you would want that, but still, just pointing out that to someone who doesn't know the functions it may still be unclear).

In summary, my vote would be that making traverseVoid as an alias for traverse_ would be a positive move without introducing a larger question of re-litigating def void.

rossabaker commented 1 month ago

I like traverseVoid, too.

Here's my question: do we deprecate the old and create toil, or live with the duplicate names until the heat death of the universe and/or Cats 3?

satorg commented 1 month ago

I think we can arrange something in the middle. For example:

  1. Add the aliases but do not deprecate the old method names.
  2. When cutting the next release, mention in the release notes, that we're going to deprecate those methods soon.
  3. After the next release create a follow-up PR that deprecates the old names, but do not merge it. Keep it unmerged until two or three releases get passed.
  4. Once we feel that the majority of downstream projects are ready, merge the PR eventually.
  5. Cut yet another release.

A plan like that might work, I guess.

joroKr21 commented 1 month ago

I think it's possible to offer a scalafix rewrite that could even be applied by scala-steward. This could be done together with deprecating the old methods.

satorg commented 1 month ago

Agree, a scalafix rule could come in handy indeed. I would suggest though to address it in a separate follow-up PR, i.e.:

  1. Introduce aliases;
  2. Create the scalafix rule and deprecate the old names.

Just to let the aliases go live without awaiting the rewrite to come out. I feel we can expedite the whole process that way.

That said, I also feel that in this particular case for many users it could be way easier to do a simple "find-and-replace" across their projects rather than bother with scalafix. Especially if there's no automated process of applying scalafix rules in there (which I bet many real-world projects don't have).