CU-m3dyef Either, Option and Validated tap effect

[raulraja]

Addresses #2416 and #2410
In the second case in #2410 tap is the same as foreach and there is also tapNone

[franciscodr]

Task linked: CU-m3dyef Review User requested functionality

[pablisco]

Just one potential suggestion. Otherwise looks good 👍

[pablisco]

We can also express this with a scoping function:

Suggested change

	public inline fun tapLeft(f: (A) -> Unit): Either<A, B> =
	when (this) {
	is Left -> {
	f(this.value)
	this
	}
	is Right -> this
	}
	public inline fun tapLeft(f: (A) -> Unit): Either<A, B> = apply {
	if(this is Left) {
	f(value)
	}
	}

Is it easier to read? 🤔

[pablisco]

If this feels better it would apply to the other taps 🙂 Otherwise happy to be ignored 😅 #notablocker

[raulraja]

@pablisco apply it's fine since I suppose the lambda has no cost, and its body is inlined since it's flagged as invoked once in its contract. I speculate it does not need to capture the state for the lambda. I'd be curious to see the resulting bytecode because the when statement I wrote has no allocations and most likely results in a table-switch indexed lookup.
In whatever case, It's not important to me. If you want to suggest some changes to the impl in the same branch, we can apply them before merging; I'm sure whatever users write in the lambda is gonna be more costly than any optimizations :)

[pablisco]

@raulraja Actually, now I think of it we could add the option to add effects for both sides of the bifunctor and make use of existing operations 🙂

  public inline fun onLeft(f: (A) -> Unit): Either<A, B> =
    onEither(left = f, right = {})

  public inline fun onRight(f: (B) -> Unit): Either<A, B> =
    onEither(left = {}, right = f)

  public inline fun onEither(left: (A) -> Unit, right: (B) -> Unit): Either<A, B> = 
    apply { fold(left, right) }

[raulraja]

that's probably allocating for each one of those lambdas? If that is the case, I'd rather have an alloc free impl that is less readable as we have no context as to how these functions will be used.

[pablisco]

That is probably right. One option may be to use a decoy for the empty lambda if we don't have something like this already. Need to check the byte code :) 
As I said, unless someone else sees this useful, we can explore it later on

[nomisRev]

I think apply is confusing here since apply is typically for mutable instantiation logic, like builder syntax.
Which this definitely doesn't do.

I think the when code Raul has is the simplest 👍

[pablisco]

Ah, I got something else. 😅  Would it make sense to call these onLeft, onRight, etc? To follow the same pattern as side effects that other components use like onEach on the stdlib and onNext on things like Rx.

[raulraja]

@pablisco I have no preference for the name of these functions and @nomisRev brought up to me he saw them useful in some context but I'm also not familiar with onX to refer to them. If the std lib already has a convention and is onX then we should probably use that but not sure.

[nomisRev]

I think the convention we've been using is ifLeft, ifRight, ifNone, etc.

onX seems to imply executing a side-effect, which is actually why this naming was chosen in ReactiveX IIRC.

EDIT:
public inline fun <C> fold(ifLeft: (A) -> C, ifRight: (B) -> C): C
https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Either.kt#L761

[nomisRev]

I think apply is confusing here since apply is typically for mutable instantiation logic, like builder syntax.
Which this definitely doesn't do.

I think the when code Raul has is the simplest 👍