Add orElse to ApplicativeError
#2243
Conversation
| @@ -70,4 +70,7 @@ final class ApplicativeErrorOps[F[_], E, A](val fa: F[A]) extends AnyVal { | |||
|
|
|||
| def onError(pf: PartialFunction[E, F[Unit]])(implicit F: ApplicativeError[F, E]): F[A] = | |||
| F.onError(fa)(pf) | |||
|
|
|||
| def or(other: F[A])(implicit F: ApplicativeError[F, E]): F[A] = | |||
There was a problem hiding this comment.
Yeah, I’d be +1 in making it lazy.
There was a problem hiding this comment.
very true, I will change it. Thanks
Codecov Report
@@ Coverage Diff @@
## master #2243 +/- ##
==========================================
- Coverage 95.05% 94.95% -0.11%
==========================================
Files 333 333
Lines 5789 5788 -1
Branches 211 221 +10
==========================================
- Hits 5503 5496 -7
- Misses 286 292 +6
Continue to review full report at Codecov.
|
|
|
|
changing it to |
|
Could we run the CI again ? Thanks |
| test("or leaves unchanged a success") { | ||
| 17.some or None should === (Some(17)) | ||
| test("orElse leaves a success unchanged") { | ||
| 17.some orElse None should === (Some(17)) |
There was a problem hiding this comment.
this test is only testing the the Option.orElsemethod.
|
I added a wrapper for option with its ApplicativeError instance as I couldn't find anything with one that didn't also have an orElse method. I know that there is Future but that makes more complex the comparison. |
| @@ -35,5 +35,31 @@ class ApplicativeErrorSuite extends CatsSuite { | |||
| failed.recoverWith { case _ => Some(7) } should === (Some(7)) | |||
| } | |||
|
|
|||
| { | |||
| final case class OptionWrapper[A](option: Option[A]) | |||
There was a problem hiding this comment.
Thanks for the quick fix. Just FYI (not saying we should introduce another dependency in cats), with https://github.com/estatico/scala-newtype, all these can be written as
@newtype case class OptionWrapper[A](option: Option[A])
object OptionWrapper {
implicit def ae[E](implicit ev: ApplicativeError[Option, E]): ApplicativeError[OptionWrapper, E] = ev.asInstanceOf
}There was a problem hiding this comment.
thanks, yes, I would love to use it 😄 , very compact indeed. May be in the near future ... 😃
|
👍 thank you |
This is to address #2242