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 ifTrueM, ifFalseM, ensureTrue and ensureFalse #4637

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

geirolz
Copy link

@geirolz geirolz commented Jul 19, 2024

This PR adds ifTrueM and ifFalseM for Monad and ensureTrue and ensureFalse for MonadError.

The reasons why they are under Monad and not Applicative is because they needs pure for the unhandled branch.

Considerations

  • ifTrueM / ifFalseM - Idk if this should stay in Monad or just in the syntax since depends on Monoid.
  • ensureTrue / ensureFalse - I was unsure about the naming between these and raiseWhenTrue/raiseWhenFalse.
  • I also thought to add these methods into mouse instead but I ended up that those cases are too common and useful to stay in mouse ( there are also other methods in mouse that IMHO are worth moving to cats, but that is another topic )

Let me know what do you think!

@geirolz geirolz marked this pull request as draft July 19, 2024 13:50
@geirolz geirolz marked this pull request as ready for review July 19, 2024 14:07
@johnynek
Copy link
Contributor

I'm concerned with Monoid being used just to get a default value and not due to any laws about monoid relating to the functions using it.

@geirolz
Copy link
Author

geirolz commented Jul 19, 2024

I'm concerned with Monoid being used just to get a default value and not due to any laws about monoid relating to the functions using it.

I see you point - I don't have strong options here. The alternative is to return Unit for both branches.

@@ -162,6 +164,18 @@ trait Monad[F[_]] extends FlatMap[F] with Applicative[F] {
tailRecM(branches.toList)(step)
}

/**
* If the `F[Boolean]` is `true` then return `ifTrue` otherwise return `ifFalse`
Copy link
Contributor

Choose a reason for hiding this comment

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

The doc doesn't match with the implementation

@satorg
Copy link
Contributor

satorg commented Jul 19, 2024

The alternative is to return Unit for both branches.

Just for reference, to keep stuff connected: #4626

I also think that instead of ifTrueWhatever and ifFalseWhatever we should use whenWhatever and unlessWhatever – it would be more consistent with other parts of both Cats and Mouse.

Comment on lines +167 to +177
/**
* If the `F[Boolean]` is `true` then return `ifTrue` otherwise return `Monoid[A].empty`
*/
def ifTrueM[B: Monoid](fa: F[Boolean])(ifTrue: => F[B]): F[B] =
ifM(fa)(ifTrue, pure(Monoid[B].empty))

/**
* If the `F[Boolean]` is `false` then return `ifFalse` otherwise return `Monoid[A].empty`
*/
def ifFalseM[B: Monoid](fa: F[Boolean])(ifFalse: => F[B]): F[B] =
ifM(fa)(pure(Monoid[B].empty), ifFalse)
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest adding these methods to the F[Boolean] syntax in Mouse https://github.com/typelevel/mouse/blob/main/shared/src/main/scala/mouse/fboolean.scala

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.

5 participants