Adding biSemiflatMap to EitherT (#2269) #2274
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2274 +/- ##
==========================================
+ Coverage 94.97% 94.97% +<.01%
==========================================
Files 337 337
Lines 5832 5835 +3
Branches 217 206 -11
==========================================
+ Hits 5539 5542 +3
Misses 293 293
Continue to review full report at Codecov.
|
| @@ -120,6 +120,9 @@ final case class EitherT[F[_], A, B](value: F[Either[A, B]]) { | |||
| case r@Right(_) => F.pure(r.leftCast) | |||
| }) | |||
|
|
|||
| def biSemiflatMap[C, D](fa: A => F[C], fb: B => F[D])(implicit F: Monad[F]): EitherT[F, C, D] = | |||
| leftSemiflatMap(fa).semiflatMap(fb) | |||
There was a problem hiding this comment.
Maybe it would be better to implement this in a single walk over F?
If F is e.g. List, this would traverse the list twice, and we should probably not do that ;)
Other than that, I think it's worth mentioning this PR to one of the maintainers, since there hasn't been any activity from them on this or the original issue.
There was a problem hiding this comment.
Fair point @kubukoz , so I would refactor it to be:
def biSemiflatMap[C, D](fa: A => F[C], fb: B => F[D])(implicit F: Monad[F]): EitherT[F, C, D] =
EitherT(F.flatMap(value) {
case Left(a) => F.map(fa(a)) { c => Left(c) }
case Right(b) => F.map(fb(b)) { d => Right(d) }
})
In this case it will flatMap and map it once, instead of flatMap it twice.
|
@kailuowang would you mind reviewing this please? |
|
Sorry about that, I just had the time to fix it now, thank you! :) Would you mind having another look @LukaJCB @kailuowang ? I've automatically dismissed your approval when I pushed :| |
* Adding biSemiflatMap to EitherT * Expanding leftSemiflatMap and semiflatMap * Added documentation * Added EitherT types for 2.10 and 2.11
Addresses #2269.