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

Deprecate ifA. #3893

Merged
merged 1 commit into from
May 30, 2021
Merged

Deprecate ifA. #3893

merged 1 commit into from
May 30, 2021

Conversation

diesalbla
Copy link
Contributor

@diesalbla diesalbla commented May 18, 2021

So, a year and a half ago, somebody thought that having an ifA method, an applicative counterpart of the ifM method from FlatMap (or M for Monad), would be a good idea. Turns out, having an "If-then-else" that actually performs both effects is hardly ever desirable, but the name naturally suggests it would not, so that was quite misleading. This has tripped some people. Thus the time has come to deprecate it, and retire it soon.

Resolves #3889

rossabaker
rossabaker previously approved these changes May 18, 2021
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

I agree with this. I like the Selective version, but we need to finish Selective.

LukaJCB
LukaJCB previously approved these changes May 18, 2021
@diesalbla
Copy link
Contributor Author

This is failing because of some problems in main branch.

[info] Formatting 1 Scala sources...
[error] /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala:19: error: identifier expected but [ found
[error]     onFlatMapped: [X] => (S[X], X => Free[S, A]) => B
[error]                   ^: /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala
[error] /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala:19: error: identifier expected but [ found
[error]     onFlatMapped: [X] => (S[X], X => Free[S, A]) => B
[error]                   ^: /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala
[error] /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala:19: error: identifier expected but [ found
[error]     onFlatMapped: [X] => (S[X], X => Free[S, A]) => B
[error]                   ^: /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala
[error] (freeJVM / Compile / scalafmt) /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala:19: error: identifier expected but [ found
[error]     onFlatMapped: [X] => (S[X], X => Free[S, A]) => B
[error]                   ^: /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala
[error] (freeJS / Compile / scalafmt) /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala:19: error: identifier expected but [ found
[error]     onFlatMapped: [X] => (S[X], X => Free[S, A]) => B
[error]                   ^: /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala
[error] (freeNative / Compile / scalafmt) /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala:19: error: identifier expected but [ found
[error]     onFlatMapped: [X] => (S[X], X => Free[S, A]) => B
[error]                   ^: /Users/diesalbla/floss/cats/free/src/main/scala-3.x/cats/free/FreeFoldStep.scala
[error] Total time: 0 s, completed 19 May 2021, 11:26:08

@rossabaker
Copy link
Member

Were those problems fixed? I see main is passing. Do we just need a merge/rebase?

The ifA method has been discussed as dangerous and misleading, because
executing the effects of _both_ branches is hardly ever desirable.
Proposed in Issue #3889

Instead, users of this method should use either the `ifM` method of
the `FlatMap` class (when each branch is an effect) or the `ifF`
method of the `Functor` class (where each branch is a value).
@diesalbla diesalbla dismissed stale reviews from LukaJCB and rossabaker via 62672b5 May 21, 2021 23:23
@diesalbla
Copy link
Contributor Author

Were those problems fixed? I see main is passing. Do we just need a merge/rebase?

Ammended, rebased, and pushed. Sadly, reviews are lost. Could you look again?

Comment on lines +228 to 229
@deprecated("Dangerous method, use ifM (a flatMap) or ifF (a map) instead", "2.6.2")
def ifA[A](fcond: F[Boolean])(ifTrue: F[A], ifFalse: F[A]): F[A] = {
Copy link
Member

Choose a reason for hiding this comment

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

We can also eventually make this package private, shutting off the source access without breaking bincompat. We should wait a while though.

@djspiewak djspiewak merged commit 3fa2ea5 into typelevel:main May 30, 2021
@diesalbla diesalbla deleted the deprecate_ifa branch June 4, 2021 23:04
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.

Deprecate and remove ifA
4 participants