typelevel / mouse

A small companion to cats
MIT License
363 stars 66 forks source link

Add asIn, voidIn, leftAsIn, leftVoidIn into foption and feither. #458

Closed geirolz closed 8 months ago

geirolz commented 9 months ago

Hi guys! This PR adds some useful methods based on mapIn and leftMapIn.

F[Option[A]]

F[Either[L, R]]

What do you think ?

benhutchison commented 8 months ago

@geirolz Are these operations actually being used in your codebases, or is it simply that they might be used? I'd feel more comfortable if we had examples in the wild where these would be used.

As these methods demonstrate, there are many possible permutations and conversions when nested contexts are involved. How many should Mouse define, vs leaving programmers to compose the operations at the use site? It's an ambiguous question with no clearly correct answer..

But extra library surface area does come with costs - slower compile times and code completion, more crowded namespaces.

benhutchison commented 8 months ago

Apologies, hit wrong button, intended to comment not close.

geirolz commented 8 months ago

Thanks for your feedback @benhutchison! I see your point and for this reason I don't want to introduce some case-specific methods. The as method comes in a context where you have a map, writing code with cats and mouse I was expecting those methods that are useful when you want to discard the result.

My specific case was that I had a service returing IO[Either[Error, EmailValidationToken]], I had to store the token into database but returing an IO[Either[Error, Unit]] as my endpoint logic. I solved with .mapIn(_ => ()) a bit verbose and If there is a map I expecting the as as well.

Cats can't introduce these useful method, If I get it right the purpose of mouse is to add extra syntax to simplify the usage of cats. I believe that these operators could be useful in some cases. Functor syntax bring the as operator and is cool that in nested context you have the same operators proxied with suffix In to avoid boilerplates.

danicheg commented 8 months ago

@benhutchison That's a good point, Ben. I agree that generating methods by permutating combinators shouldn't be the way of evolving the mouse library. It's much more desirable to have handy generic-like combinators by using that users could satisfy their needs. But what about these particular suggested methods in this PR, I really think they might find applicability in the real world. F[Option[Unit]] is something like a cancellation token and could be found in the Cats-Effect library. Correspondingly, F[Either[Error, Unit] is a type for computations that I've seen many times in my career. Error here could imply some error type encoded via ADT.

benhutchison commented 8 months ago

@danicheg I'm happy to defer to your judgment here. I don't want to get in the way of programmers factoring out repeated structure in their code; I believe it's important to keep things DRY. Mouse is the blessed place for these sorts of combinators.

I just want to ensure we are harvesting actual shared structure from users, and don't verge into speculating shared structures that conceivably might be useful.

@geirolz I'd have been more comfortable if the PR mentioned something about your usage of the proposed operations.

Thanks for detailing the IO[Either[Error, EmailValidationToken]] example as a motivator for voidIn on Either, built on top of asIn.

🤔

geirolz commented 8 months ago

I just want to ensure we are harvesting actual shared structure from users, and don't verge into speculating shared structures that conceivably might be useful.

Makes sense and I agree with you on this!

and then the combinators for Option come by extension of the idea?

Basically yes, voidIn is built on top of asIn which is built on top of mapIn, since mapIn is available on both foption and feither as a user i would expect these method on both. Some times I've seen something like F[Option[Boolean]] to encode the case where:

In this case I may want to ignore the result and return another type.

// some(true) => deleted
// some(false) => not-deleted
// none => not-found
def deleteTempUserInfo(userId: UserId): IO[Option[Boolean]]

// I just care if the user exists ( to return a 404 for instance ) but I don't really case about the result 
val result: IO[Option[UserId]] = deleteTempUserInfo(userId).asIn(userId)

and then the left combinators by further extension?

Yes but I got a case where I wanted to replace the very specific error ( Left ) with a generic HTTP one like Forbidden.

benhutchison commented 8 months ago

OK, thanks for the explanation 👍