fix bind logic for Option to raise the transformed value.

[freynder]

Current version just returns the transform, however None is the error situation and should trigger a raise. This changes the API.

[nomisRev]

Hey @freynder,
We spoke and Slack and I am afraid I was too fast to respond 🙈 My apologies.

Looking at this signature, with this implementation I am actually wondering if it makes sense at all. So first of all thank you to raise this PR, and thoughts on the signature 🙏

I am going to rubber duck my thoughts, and ask some feedback from some fellow contributors at the same time.

The lambda Raise<R>.() -> A allows to bind, or unwrap, Option and in the case None is encountered it will invoke the lambda. The lambda allows to raise a value of R into the outer interface Raise<R>, or fallback to a value of A.

1. The Raise<R> lambda receiver is redundant because we're already inside the context of interface Raise<R>, so even without the lambda receiver we can call raise(r).
2. Without Raise<R>, we have () -> A which matches the already existing Option API of getOrElse.
3. Since this function is defined inside of an interface, it cannot be inline so it's inferior to the already existing getOrElse which is inline and thus allows for suspend to pass through.

So this API can be completely replaced by getOrElse and simplifies the examples of the KDoc above.

suspend fun test() {
  val empty: Option<Int> = None
  either {
-    val x: Int = empty.bind { _: None -> 1 }
+    val x: Int = empty.getOrElse { 1 }
-    val y: Int = empty.bind { _: None -> raise("Something bad happened: Boom!") }
+    val y: Int = empty.getOrElse { raise("Something bad happened: Boom!") }
-    val z: Int = empty.recover { _: None ->
+    val z: Int = empty.getOrElse {
      delay(10)
      1
-    }.bind { raise("Something bad happened: Boom!") }
+   }
    x + y + z
  } shouldBe Either.Left("Something bad happened: Boom!")
}

As the example shows we can now call suspend fun delay directly into the getOrElse lambda, and we can also still call raise & co from inside the getOrElse lambda.

The same applies for Result#bind. Since none of this code has yet been released, we can still remove these APIs in favour of the simpler inline methods on the original APIs. Fixing this issue also requires some other small changes in Builders.kt.

If you're interested @freynder I'd be happy to guide you through all the changes ☺️ We can discuss further on Slack, or here.

cc\ @serras @franciscodr @raulraja

[freynder]

Hello @nomisRev,

No problem, I hadn't looked at this from an API perspective, just wanted to make it consistent with the Either.bind() and others, they also do raise on error.

I fully agree with your assessment though, it would make the API much simpler. My only concern is that the bind() functions provided a consistent way for all monads to raise (and thus short-circuit) the program. getOrElse makes the raise call optional. I don't have enough knowledge currently to determine if that's important though.

Thank you for your great feedback, very much appreciated.

[freynder]

Hello,
Looking further at the code, I also noticed the same reasoning related to your point "1." can be applied to Raise.catch:

@RaiseDSL
public inline fun <R, A> Raise<R>.catch(
  @BuilderInference action: Raise<R>.() -> A,
  @BuilderInference catch: Raise<R>.(Throwable) -> A,
): A {
  contract {
    callsInPlace(action, EXACTLY_ONCE)
    callsInPlace(catch, AT_MOST_ONCE)
  }
  return fold({ action(this) }, { this@catch.catch(it) }, { this@catch.raise(it) }, { it })
}

Here the catch function receiver also seems redundant as it does not need the Raise receiver since it can access it from the this@catch Raise scope.
A counter argument to this may be that the caller may want to use a predefined lambda variable as a handler for the catch function with receiver to receive the context. Not sure how common that would be though, personally I would prefer to be explicit.
Probably also same remark for recover

[nomisRev]

Hey @freynder,
This is great feedback and I was actually also looking at this. There is multiple benefits to this actually, and we should review the current signatures in arrow.core.raise.* to see how we can simplify them.

Looking for example at recover:

@RaiseDSL
- public inline fun <R, E, A> Raise<R>.recover(
+ public inline fun <E, A> recover(
  @BuilderInference action: Raise<E>.() -> A,
-  @BuilderInference recover: Raise<R>.(E) -> A,
+  recover: (E) -> A,
): A {
  contract {
    callsInPlace(action, EXACTLY_ONCE)
    callsInPlace(recover, AT_MOST_ONCE)
  }
  return fold<E, A, A>({ action(this) }, { throw it }, { recover(it) }, { it })
}

Besides still supporting the current use-cases it also makes this function more general purpose, because you can now use it to execute Raise<E> based programs and resolve the error completely without having to stick to Raise<E>.

So thanks again for raising this issue, and PR. I think it will result in another significant improvement, and simplification of this package.

[freynder]

Hey @nomisRev,

Wow, that indeed removes the Raise<R> receiver completely. Loving how elegant this is implemented with the fold function actually creating an isolated raise context and using the general callbacks to transform / act on the possible outcomes.

Looking at it from the above perspective, recover and catch are basically predefined behaviors for fold. That makes the mental model much easier for me.

• fold: run action, general callbacks for exception, recover, transformation
• fold: run action, rethrow exception, general callbacks for recover, transformation
• recover: run action, rethrow exception, recover from raise, identity transformation
• catch: run action, catch exception, re-raise raise, identity transformation

Thank you!

[nomisRev]

Hey @freynder,

That is absolutely correct. I might steal that phrasing, and those bullet points for the new documentation 😁
I am going to close this PR, since it's preceded by #2954.

Thank you again for triggering this simplification, and discussion! Glad it helped you get a better mental model of this encoding as well.