ensureNotNull-alike binding scope extensions

[kirillzh]

Arrow has a handy extension for eagerly returning from effect with an error if a value is null - ensureNotNull. Would be very helpful to have a similar extension here, right now need to rely on fairly nested flatMap:

val value = maybeGetValue()
  .flatMap {
    when (it) {
      null -> Err(ValueMissingError)
      else -> Ok(it)
    }
  }
  .bind()

vs

val value = maybeGetValue().bind()
ensureNotNull(value) { ValueMissingError }

[michaelbull]

I may be misunderstanding, but I think we already support this behaviour with toResultOr.

Your example would go like this:

val value = maybeGetValue()?.toResultOr { ValueMissingError }.bind()

[michaelbull]

@kirillzh does that help?

[kirillzh]

To clarify, maybeGetValue in this example would return a Result with nullable value, for example `Result<String?, SomeError>.

I guess we can't quite use toResultOr() directly on the result here, bit we could chain bind()s: 🤔

val value = maybeGetValue()
  .bind()
  .toResultOr { ValueMissingError }
  .bind()

[michaelbull]

Ah okay, so you want to keep it as a Result but turn it to an Err if the current Result is an Ok(null)? Is my understanding here correct?

[kirillzh]

Exactly! The original proposed API for that I mentioned...

val value = maybeGetValue().bind()
ensureNotNull(value) { ValueMissingError }

Now as I'm thinking through it, the actual API that I am looking for roughly looks like this:

inline fun <V, E> Result<V?, E>.ensureNotNull(
  error: (V?) -> E,
): Result<V, E> = ensure(
  predicate = { it != null },
  error = error,
).map { it as V }

inline fun <V, E> Result<V, E>.ensure(
  predicate: (V) -> Boolean,
  error: (V) -> E,
): Result<V, E> = flatMap { if (predicate(it)) Ok(it) else Err(error(it)) }

And so instead of using flatMap:

val value = maybeGetValue()
  .flatMap {
    when (it) {
      null -> Err(ValueMissingError)
      else -> Ok(it)
    }
  }
  .bind()

Can use ensureNotNull here:

val value = maybeGetValue()
  .ensureNotNull { ValueMissingError }
  .bind()

[michaelbull]

I wonder if we just make a toErrorIfNull that utilises the toErrorIf function? Same with toErrorUnlessNull.

[kirillzh]

@michaelbull, toErrorIfNull is great, thanks for adding! One more thing though, could we make it return result with non-nullable value?

Right now it's implemented as:

public inline fun <V, E> Result<V, E>.toErrorIfNull(error: () -> E): Result<V, E> {

Which returns nullable success value:

val account = accountRepository.getActiveAccount()
  .toErrorIfNull { AccountMissing }
  .get() // Account? <--- nullable

A preferred signature here would be:

inline fun <V, E> Result<V?, E>.toErrorIfNull(error: () -> E): Result<V, E>

Which would return non-nullable success value:

val account = accountRepository.getActiveAccount()
  .toErrorIfNull { AccountMissing }
  .get() // Account <--- non-nullable

[michaelbull]

@kirillzh that example signature doesn't seem to work for me:

[kirillzh]

get() is always expected to be nullable, in case if result is an error, right? The benefit is that toErrorIfNull itself returns Result<String, Int> - so map would transform over String and not String? for example.

[michaelbull]

So what is the change you are requesting?

[kirillzh]

@michaelbull, consider this snippet:

fun provideResult(): Result<String?, Int> = Ok(null)

fun main() {
  provideResult()
    .toErrorIfNull { 50 }
    .map { value ->
      value.length // <- won't compile, value is nullable
    }
}

I would expect for toErrorIfNull() to return Result<String, Int> not Result<String?, Int>.

[michaelbull]

Is there anything more that I need to do other than change the signature to facilitate that example that you've posted?

[kirillzh]

I think it needs to change from this:

public inline fun <V, E> Result<V, E>.toErrorIfNull(error: () -> E): Result<V, E> {
    contract {
        callsInPlace(error, InvocationKind.AT_MOST_ONCE)
    }

    return toErrorIf(
        { it == null },
        { error() }
    )
}

To this:

public inline fun <V, E> Result<V?, E>.toErrorIfNull(error: () -> E): Result<V, E> {
  contract {
    callsInPlace(error, AT_MOST_ONCE)
  }

  return toErrorIf(
    { it == null },
    { error() }
  ).map {
    @Suppress("UNCHECKED_CAST")
    it as V
  }
}

[michaelbull]

Okay, I think writing it out as a when and not utilising toErrorIf/map will probably end up being the play here as it'll likely improve readability and potentially avoid a cast. Will give it a go for both this and the unless case as well.

[kirillzh]

@michaelbull, consider this snippet:

fun provideResult(): Result<String?, Int> = Ok(null)

fun main() {
  provideResult()
    .toErrorIfNull { 50 }
    .map { value ->
      value.length // <- won't compile, value is nullable
    }
}

I would expect for toErrorIfNull() to return Result<String, Int> not Result<String?, Int>.

Addressing this in #86