-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
added leftT and improved existing lift API for EitherT #1614
added leftT and improved existing lift API for EitherT #1614
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1614 +/- ##
==========================================
+ Coverage 93.08% 93.12% +0.04%
==========================================
Files 250 250
Lines 3989 3999 +10
Branches 136 135 -1
==========================================
+ Hits 3713 3724 +11
+ Misses 276 275 -1
Continue to review full report at Codecov.
|
Thank you for this @kailuowang |
|
||
final def right[F[_], A, B](fb: F[B])(implicit F: Functor[F]): EitherT[F, A, B] = EitherT(F.map(fb)(Either.right)) | ||
final class LeftPartiallyApplied[B] private[EitherTFunctions] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious. Is Scalaz's trick here (add a dummy parameter to this class to make it an AnyVal
; see here) necessary to make this zero-cost?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems a nice trick. Do we want to use it here first in this PR, or create another PR applying the pattern globally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with another PR to apply it globally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
* res1: EitherT[List, String, Int] = EitherT(List(Right(42))) | ||
* }}} | ||
*/ | ||
final def fromOptionF[F[_], E, A](fopt: F[Option[A]], ifNone: => E)(implicit F: Functor[F]): EitherT[F, E, A] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering, is this better than OptionT(fopt).toRight(ifNone)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't know about the OptionT.toRight
, Looks like this one has slightly less function calls and an allocation to OptionT
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterneyens anything else in this PR?
using PartiallyApplied to improve the API
added a
leftT
andrightT
alias