Allow to recover to Option<A> in Option.catch

[dnowak]

The current version of catch that has (Throwable) -> Unit as recover action does not allow to handle the exception in any usable way. The only possible action is a side effect (ie. to log the exception).

I think that we should have the possibility to recover from Throwable to Opton<A> -> recover type should be (Throwable) -> Option<A>. This is much more useful than executing some side effect.

I have changed the current function. Maybe it should be deprecated and a new one added?

[nomisRev]

Suggested change

	inline fun <A> catch(recover: (Throwable) -> Option<A>, f: () -> A): Option<A> =
	@JvmOverloads
	inline fun <A> catch(recover: (Throwable) -> Option<A> = { None }, f: () -> A): Option<A> =

This channge would not cause any binary breaking change due to the default argument and @JvmOverloads creating a second method with the original signature.

I'm wondering if we should keep the original function, or if we should be more strict like in this PR.

[raulraja]

I think we should keep the original function as well. It's useful in suspend code?

[dnowak]

Hi, @nomisRev, there could be 2 ways of handling exceptions for Option. The library user would decide which version to use. But the names for JVM have to be different. I got this error when I had 2 versions of the catch function

git/arrow/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Option.kt: (331, 14): Platform declaration clash: The following declarations have the same JVM signature (tryCatch(Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function0;)Larrow/core/Option;):
    fun <A> tryCatch(recover: (Throwable) -> Option<A#1 (type parameter of arrow.core.Option.tryCatch)>, f: () -> A#1): Option<A#2 (type parameter of arrow.core.Option.Companion.catch)> defined in arrow.core.Option
    fun <A> tryCatch(recover: (Throwable) -> Unit, f: () -> A#3 (type parameter of arrow.core.Option.tryCatch)): Option<A#4 (type parameter of arrow.core.Option.Companion.catch)> defined in arrow.core.Option

I have no good name for my variant :-(.

[nomisRev]

Hey @dnowak,

Damn, that's too bad that it conflicts :(

What about the following?
It deprecates the current function and replaces it with the one you proposed. Alternatively, it also adds the = { None } variant but as an explicit overload rather than as an automatically generated one.
That way we can use tryCatchOrNone as the JVM name instead.

@JvmStatic
@JvmName("tryCatch")
@Deprecated(
  "Side-effecting recover which loses Throwable is no longer supported",
  ReplaceWith("Option.catch({ recover(it); None }, f)")
)
inline fun <A> catch(recover: (Throwable) -> Unit, f: () -> A): Option<A> =
  try {
    Some(f())
  } catch (t: Throwable) {
    recover(t.nonFatalOrThrow())
    None
  }

@JvmStatic
@JvmName("tryCatchOrNone")
inline fun <A> catch(f: () -> A): Option<A> =
  try {
    Some(f())
  } catch (t: Throwable) {
    t.nonFatalOrThrow()
    None
  }

@JvmStatic
@JvmName("tryCatch")
inline fun <A> catch(recover: (Throwable) -> Option<A>, f: () -> A): Option<A> =
  try {
    Some(f())
  } catch (t: Throwable) {
    recover(t.nonFatalOrThrow())
  }

[dnowak]

Hi @nomisRev ,

The deprecated variant also needs a specific name for Java. I propose catchOrSideEffect. It will change Java API. I do not know how important is the stability of Java API. Anyway here is my current proposal:

    @JvmStatic
    @JvmName("tryCatchOrSideEffect")
    @Deprecated(
      "Side-effecting recover which loses Throwable is no longer supported",
      ReplaceWith("Option.catch({ recover(it); None }, f)")
    )
    inline fun <A> catch(recover: (Throwable) -> Unit, f: () -> A): Option<A> =
      try {
        Some(f())
      } catch (t: Throwable) {
        recover(t.nonFatalOrThrow())
        None
      }

    @JvmStatic
    @JvmName("tryCatchOrNone")
    /**
     * Ignores exceptions and returns None if one is thrown
     */
    inline fun <A> catch(f: () -> A): Option<A> {
      val recover: (Throwable) -> Option<A> = { None }
      return catch(recover, f)
    }

    @JvmStatic
    @JvmName("tryCatch")
    inline fun <A> catch(recover: (Throwable) -> Option<A>, f: () -> A): Option<A> =
      try {
        Some(f())
      } catch (t: Throwable) {
        recover(t.nonFatalOrThrow())
      }

PS. How to add such a nice block of code into a comment? :-)

[AzimMuradov]

PS. How to add such a nice block of code into a comment? :-)

Hey @dnowak, try to add "kotlin" right after the first ```, like this:

```kotlin

your code

```

You can find full docs here.

[dnowak]

Thanks

[nomisRev]

Sorry, for the late response @dnowak. Thanks, @AzimMuradov.

[nomisRev]

Thank you for making this PR and contribution! 🙌 This was definitely missing!!

[nomisRev]

Thank you for the contributions @dnowak! 🙌 🙌
This will be included in the 1.0.0-M1 release soon ;)

[raulraja]

thanks @dnowak !

[raulraja]

I think we should keep the original function as well. It's useful in suspend code?