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

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

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

geirolz
Copy link
Contributor

@geirolz geirolz commented Oct 9, 2023

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

F[Option[A]]

  • asIn -> map F[Option[A]] to F[Option[B]] ignoring the A value.
  • voidIn -> drain F[Option[A]] to F[Option[Unit]] ignoring the A value.

F[Either[L, R]]

  • asIn -> map F[Either[L, R]] to F[Either[L, B]] ignoring the R value.
  • voidIn -> drain F[Either[L, R]] to F[Either[L, Unit]] ignoring the R value.
  • leftAsIn -> map F[Either[L, R]] to F[Either[B, R]] ignoring the L value.
  • leftVoidIn -> drain F[Either[L, R]] to F[Either[Unit, R]] ignoring the L value.

What do you think ?

Copy link
Member

@danicheg danicheg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! I think it'd be convenient to have these suggested methods 👍🏻

shared/src/test/scala/mouse/FEitherSyntaxTest.scala Outdated Show resolved Hide resolved
@benhutchison
Copy link
Member

@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
Copy link
Member

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

@geirolz
Copy link
Contributor Author

geirolz commented Oct 11, 2023

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
Copy link
Member

@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
Copy link
Member

benhutchison commented Oct 12, 2023

@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.

🤔

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

@geirolz
Copy link
Contributor Author

geirolz commented Oct 13, 2023

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:

  • None => Operation not executed
  • Some(true) => Operation executed successfully
  • Some(false) => Operation failed, error it's not really important

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
Copy link
Member

OK, thanks for the explanation 👍

@benhutchison benhutchison merged commit 0d5d6a2 into typelevel:main Oct 13, 2023
26 checks passed
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

Successfully merging this pull request may close these issues.

3 participants