-
Notifications
You must be signed in to change notification settings - Fork 451
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 MaybeK wrapper for RxJava Maybe #862
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.
Dooocs
Yeees I'll add docs this weekend, but need your help on those folds :D |
IIRC Maybe has the same semantics as Observable, except it's max 1 return. |
Given that On |
There you got the docs also, back to you @pakoito |
Codecov Report
@@ Coverage Diff @@
## master #862 +/- ##
============================================
+ Coverage 44.85% 45.15% +0.29%
- Complexity 628 640 +12
============================================
Files 300 302 +2
Lines 7709 7763 +54
Branches 834 837 +3
============================================
+ Hits 3458 3505 +47
- Misses 3947 3954 +7
Partials 304 304
Continue to review full report at Codecov.
|
fun <B> flatMap(f: (A) -> MaybeKOf<B>): MaybeK<B> = | ||
maybe.flatMap { f(it).fix().maybe }.k() | ||
|
||
fun <B> fold(ifEmpty: () -> B, ifSome: (A) -> B): B = when { |
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.
According to RxJava's docs, this.maybe.blockingGet()
returns null
for no value and A
for a value. Given that RxJava2 doesn't allow nulls in Maybe
, you can rewrite this to be simpler:
maybe.blockingGet().let {
if (it == null) ifEmpty() else ifSome(if)
}
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 would suggest adding a new test for it too.
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'll address both comments tomorrow. Out of home today, Champions League finals tonight 💥
If you're hurry for a release, feel free to push those changes. Otherwise I'll do.
Done and done @pakoito 👍 |
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.
Let's mergeee!
👍 |
* add MaybeK * add MaybeK instances * add MaybeK tests including law tests
Fixes #860
MaybeK
@pakoito I've added
fold()
,foldLeft
,foldRight
etc to it since I saw those where also defined in other "equivalent" data types like Option and maybe also represents code branching. Also aFoldable
instance. But to be honest I'm not 100% sure the code on those methods / instance is correct, butFoldableLaws
are passing, which seems suspicious. Could you take a look?I've not added
Traverse
instance for it yet since traverse uses to rely on fold and I'm still not confident with my implementation.