-
-
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 Bitraverse for Ior (#2144) #2159
Conversation
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.
Thank you very much for your contribution, this is great! Just one small request that we use explicit pattern matching instead of fold
for performance reasons :)
implicit val catsBitraverseForIor: Bitraverse[Ior] = new Bitraverse[Ior] { | ||
|
||
def bitraverse[G[_], A, B, C, D](fab: Ior[A, B])(f: A => G[C], g: B => G[D])(implicit G: Applicative[G]): G[Ior[C, D]] = | ||
fab.fold( |
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.
Can we replace this fold
with an explicit pattern match?
) | ||
|
||
def bifoldLeft[A, B, C](fab: Ior[A, B], c: C)(f: (C, A) => C, g: (C, B) => C): C = | ||
fab.fold(f(c, _), g(c, _), (a, b) => g(f(c, a), b)) |
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.
Same here.
fab.fold(f(c, _), g(c, _), (a, b) => g(f(c, a), b)) | ||
|
||
def bifoldRight[A, B, C](fab: Ior[A, B], c: Eval[C])(f: (A, Eval[C]) => Eval[C], g: (B, Eval[C]) => Eval[C]): Eval[C] = | ||
fab.fold(f(_, c), g(_, c), (a, b) => g(b, f(a, c))) |
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.
and here.
cc411db
to
14de896
Compare
Thanks @LukaJCB, done |
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.
Thank you @V-Lamp Looks great :)
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.
Thanks! on build green
Codecov Report
@@ Coverage Diff @@
## master #2159 +/- ##
==========================================
- Coverage 94.67% 94.64% -0.03%
==========================================
Files 328 328
Lines 5538 5548 +10
Branches 199 213 +14
==========================================
+ Hits 5243 5251 +8
- Misses 295 297 +2
Continue to review full report at Codecov.
|
Added
Bitraverse[Ior]
, as discussed in #2144For the testing part, I followed the pattern found in for testing
Bitraverse[Either]
, withincats.tests.EitherSuite
This is my first PR, let me know if I need do something around this.
...and thanks for all the good work!