<@U4UGS5FC7> <@U0RM4EPC7> I’ve raised this PR to address some (not all) of the missing option extens...

# arrow-contributors

[mitch]

@raulraja @simon.vergauwen I’ve raised this PR to address some (not all) of the missing option extensions that we had to introduce internally https://github.com/arrow-kt/arrow/pull/2397 if we can get these (esp. the traverses) to be supported natively in arrow, that’s going to be massive for us!

[raulraja]

Looks great @mitch, thanks so much for taking the time 🙂

[mitch]

Awesome thanks @raulraja, I've addressed your comments. Going to wait for others to review as well.

[pablisco]

Nice job 🙂
I have a question/thought for @raulraja and @simon.vergauwen. It’s already reviewed so may not warrant any changes.
Some of the Option operations can be expressed as nullableOp.toOption() , would this be better or worse than duplicating logic? Duplicates aren’t always bad 😅
Let me know what you think 🙂

[raulraja]

I think given we accepted Option back and people use it in places where nullable types can’t be used we should accept it as first class citizen and offer the same set of apis as other data types.

[raulraja]

So it’s not so much to me a problem of duplication but consistency. If you as a user use Either, or nullable types and then at some point you have to use Option you’d expect the same set of available methods where posible so the API and coding experience is consistent.

[pablisco]

Sorry @raulraja I think I didn’t explain myself well. I don’t mean to not have these but rather if we should, internally, define them as:

inline fun <C> traverseOption(
    fa: (B) -> Option<C>,
  ): Option<Either<A, C>> =
    traverse(fa).toOption()

or as extension functions:
inline fun <A, B, C> Either<A, B>.traverseOption(
fa: (B) -> Option<C>,
): Option<Either<A, C>> =
traverse(fa).toOption()
🙂

[raulraja]

I see, the general consensus was that whenever it’s possible regular members is favorable to users because it does not impose an additional import. Too many extension imports has been a problem to arrow and the only reason why we have in some cases extensions is due to the nature of contravariance and how kotlin treats it in members.

For example it’s not possible to implement Either.flatMap as a member without using @UnsafeVariance or similar hacks but it’s possible to implement it as an extension because in extensions type arguments are invariant and spelled out in the function.

[pablisco]

Ignore the extension fun 😅 Shouldn’t have mixed matters hehe. The first snippet is the important one

[raulraja]

you mean that in your example traverse(fa) returns a nullable value and the you convert it to option?

[raulraja]

if it follows the laws of the operation and it’s faster than whatever other way is implemented then yes. As a general rule of thumb for the impls we try to follow whatever is faster and still correct no matter what it takes.

[pablisco]

So, to the end user would be the same but we save some code on our side. Nullable is “inlined” i.e. has no wrapper. But, as I said, it’s not always better to remove duplicates, depends on the case

[pablisco]

All that said, LGTM 🙂