Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

["Request"] more readable alternative to zipOrAccumulate using Kotlin contracts #3066

Closed
Intex32 opened this issue Jun 21, 2023 · 6 comments
Closed
Labels
2.0.0 Tickets / PRs belonging to Arrow 2.0

Comments

@Intex32
Copy link

Intex32 commented Jun 21, 2023

What version are you currently using? V1.2

What would you like to see?
I want to have a more readable alternative to zipOrAccumulate, when doing error accumulation.

either {
    val a: ValidatedNel<String, String> = "coo".validNel()
    val b: ValidatedNel<String, String> = "kie".validNel()
    val c: ValidatedNel<String, String> = "mesomerism".validNel()

    ensureAllValid(a, b)

    a.value
    b.value
    //c.value // does not compile

    a.value + b.value
} shouldBeEqual "cookie".right()

Motivation:
Currently, zipOrAccumulate works by taking in a number of lambdas whose return values are zipped using another trailing lambda (happy path).
Arrow Docs
zipOrAccumulate's flaws are:

  • much wrapping with lambdas
  • potential "redeclaration" of validated variables in the zip function
  • happy path is indented (less readable)

Furthermore this aligns well with current Either DSL (e.g. ensure).

Suggestion:
Use Kotlin contracts to declare a certain variables as valid if they are otherwise accumulate and combine their errors.

Concrete solution's implementation:

suspend inline fun <ERROR, ACC_ERROR, reified A, reified B> EffectScope<ACC_ERROR>.ensureAllValid(a: ValidatedNel<ERROR, A>, b: ValidatedNel<ERROR, B>, noinline mapAccErrors: (Nel<ERROR>) -> ACC_ERROR) {
    contract {
        returns() implies (a is Valid<A> && b is Valid<B>)
    }
    ensureAllValidInternal(nonEmptyListOf(a, b), mapAccErrors)
}

suspend inline fun <ERROR, reified A, reified B> EffectScope<Nel<ERROR>>.ensureAllValid(a: ValidatedNel<ERROR, A>, b: ValidatedNel<ERROR, B>) {
    contract {
        returns() implies (a is Valid<A> && b is Valid<B>)
    }
    ensureAllValidInternal(nonEmptyListOf(a, b), ::identity)
}

@PublishedApi
internal suspend fun <ERROR, ACC_ERROR> EffectScope<ACC_ERROR>.ensureAllValidInternal(
    all: Nel<ValidatedNel<ERROR, Any?>>,
    mapAccErrors: (Nel<ERROR>) -> ACC_ERROR,
) {
    val invalids = all.filterIsInstance<Invalid<Nel<ERROR>>>().toNonEmptyListOrNone()
        .getOrElse { return }
    val accumulatedErrors = invalids
        .flatMap { it.value }
        .let(mapAccErrors)
    shift<Nothing>(accumulatedErrors)
}

This of course would need to be repetitively scaled to a certain number of arguments. The example given consider just two arguments.

Current Limitations:

  • contracts still experimental
  • contracts are unstable and unreliable at the time being
  • in some cases I had compiler errors and had to recompile the entire project (I haven't figured out in detail yet, what the exact cause is)
@nomisRev nomisRev added the 2.0.0 Tickets / PRs belonging to Arrow 2.0 label Jun 29, 2023
@serras
Copy link
Member

serras commented Jul 6, 2023

If I understand correctly, what you want here is kind of "accumulate as much as possible" scenario. The shape of zipOrAccumulate ensures that all the validations can be independently executed, but this is no longer possible to guarantee if we're just using a block.

@Intex32
Copy link
Author

Intex32 commented Jul 8, 2023

@serras I'm afraid you didn't get the point. The observed behavior of my proposal should be the same as for zipOrAccumulate, just another syntax so to speak. After the ensureAllValid call - as the name suggests - all variables passed as parameters are guaranteed to be Valid. Also, in what sense do you mean independent?

@Zordid
Copy link
Contributor

Zordid commented Aug 2, 2023

As Validated has been deprecated and will be removed, I think your example could be rewritten using EitherNel, right? And ensureAllRight would be your magic ingredient for that one point where all Eithers are checked. Maybe it should more be like a bindAll? But that one would return a bunch of values as a return value which could be destructured?

@Intex32
Copy link
Author

Intex32 commented Aug 3, 2023

@Zordid
Greetings from Munich,
Yes, as Validated is deprecated, this can be transferred to EitherNel, afaik.

bindAll is slightly different. All Iterable's elements would have to be of type A which is a heavy restriction. Also, destructuring implies redeclaring all variables which often times (at least for me) results in duplicate names (one for the original and one for the validated). Thus, I view my proposal as much more convenient.

image

@serras
Copy link
Member

serras commented Jun 14, 2024

We've managed to create a DSL in this style in #3436

@serras
Copy link
Member

serras commented Oct 2, 2024

I'm closing this since #3436 is now merged. Don't hesitate to reopen if you think we can improve even more.

@serras serras closed this as completed Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0.0 Tickets / PRs belonging to Arrow 2.0
Projects
None yet
Development

No branches or pull requests

4 participants