-
Notifications
You must be signed in to change notification settings - Fork 7
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 traverse, zip and orEmpty to Option #53
Conversation
f14708e
to
c74d05e
Compare
c74d05e
to
71ae47f
Compare
5.some().zipOp("apple".some()) { a: Int, b: String -> | ||
b.length * a | ||
} shouldBeSome 25 | ||
} |
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.
} | |
} |
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 think the indent on line 46 is actually correct (2).
But the following two tests have incorrect indent (4).
inline fun <E, A, B> Option<A>.traverseOp(fa: (A) -> Either<E, B>): Either<E, Option<B>> = fold( | ||
{ None.right() }, | ||
{ fa(it).map(::Some) } | ||
) |
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.
It's unfortunate we have to use traverseOp
and zipOp
, that really reduces the value of back-porting these.
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.
traverseEither
and zipOption
would be more consistent with the variants on NonEmptyList
(which have recently been back-ported to quiver
).
We could then add traverse
and zip
when arrow
finally removes them?
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 feel like using straight traverse
and zip
is worth the wait. Let's lean on Arrow to fully remove them.
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.
Agreed. I'll open a new PR with just orEmpty
then if you're happy with it
This PR adds three new extension methods to Option:
traverseOp
traverse
is currently shadowed in Arrow, so I had to rename itMonadOption with in the context of an Either, and turn yourOption<A>
into anEither<None, Option<B>>
zipOp
map
Option<A>
with aOption<B>
and return anOption<C>
orEmpty
is a convenience function that we've been using in our codebases, that will return an empty string if your Option isNone
Option<String>
context. Thoughts?