-
-
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
Add Foldable extension collectFirstSomeM #2366
Add Foldable extension collectFirstSomeM #2366
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2366 +/- ##
==========================================
+ Coverage 95.01% 95.01% +<.01%
==========================================
Files 349 349
Lines 5998 6003 +5
Branches 222 222
==========================================
+ Hits 5699 5704 +5
Misses 299 299
Continue to review full report at Codecov.
|
c790418
to
743f0f9
Compare
* }}} | ||
*/ | ||
def collectFirstSomeM[G[_], B](f: A => G[Option[B]])(implicit F: Foldable[F], G: Monad[G]): G[Option[B]] = | ||
F.foldRight(fa, Eval.now(OptionT.none[G, B]))((a, lb) => |
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.
Would it be possible to implement this without OptionT
? I think there's some overhead involved when wrapping values in monad transformers, and this is library code so it shouldn't be that problematic to use the wrapped values directly
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.
Sure, it would. Will do if you think it's worth it.
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.
Yeah let's try to do it as efficiently as possible :)
def collectFirstSomeM[G[_], B](f: A => G[Option[B]])(implicit F: Foldable[F], G: Monad[G]): G[Option[B]] = | ||
F.foldRight(fa, Eval.now(G.pure(Option.empty[B])))((a, lb) => | ||
Eval.now(G.flatMap(f(a)) { | ||
case s @ Some(_) => G.pure(s) |
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.
We could also match on None
first and then add case s
- we'll avoid an instance check and cast, so it could be good for performance. Not sure if the difference is significant though, we'd need to benchmark. Feel free to ignore this comment ;)
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.
Got it. Let me perform some jmh benchmarks.
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.
@kubukoz JMH proved you right :)
F.foldRight(fa, Eval.now(OptionT.none[G, B]))((a, lb) => | ||
Eval.now(OptionT(f(a)).orElse(lb.value)) | ||
).value.value | ||
F.foldRight(fa, Eval.now(G.pure(Option.empty[B])))((a, lb) => |
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.
👍
What's the preferred way to restart a failed Travis build if no file changes needed? push force? |
Add an empty commit, I guess. `git commit --allow-empty`
…On Sat, Aug 11, 2018, 15:01 λoλcat ***@***.***> wrote:
What's the preferred way to restart a failed Travis build if no file
changes needed? push force?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2366 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA2npAVzu77TCEYH4V_jiyeDIQxnDMBNks5uP1QcgaJpZM4VvkBw>
.
|
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!
* Add Foldable extension collectFirstSomeM * Implement without OptionT * Optimize pattern matching * Empty commit
Monadic version of
collectFirstSome
. Implemented as a syntax extension for binary compatibility.