michaelbull / kotlin-result

A multiplatform Result monad for modelling success or failure operations.
ISC License
1.02k stars 57 forks source link

Consider adding zipOrAccumulate? #89

Closed YuitoSato closed 9 months ago

YuitoSato commented 1 year ago

Have you considered implementing a zipOrAccumulate method, which unlike zip, takes all of the Err from the Result arguments and returns them as Result<T, List<Err>>?

zip returns the first Err from the Result arguments, which can be inconvenient when we want to return multiple errors during validation. For instance, when creating a user instance, if both the name and email address are invalid, we would want to return two errors.

class User(val name: Name, val email: Email) {
  companion object {
    fun create(name: String, email: String): Result<User, List<UserCreateError>> {
      ...
    }
  }
}

Arrow's Either has a zipOrAccumulate method that can solve this problem.

https://arrow-kt.io/learn/typed-errors/working-with-typed-errors/#accumulating-different-computations https://github.com/arrow-kt/arrow/blob/cb033637ab6b6f1f134b1924c948d6638cc5acf4/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Either.kt#L1499

As the implementation is not overly complicated, would you consider including it in kotlin-result?

Here's an example implementation: ZipOrAccumulate.kt

/**
 * Apply a [transformation][transform] to ten [Results][Result], if both [Results][Result] are [Ok].
 * If not, the all arguments which are [Err] will propagate through.
 */
public inline fun <T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, E, Z> zipOrAccumulate(
    producer1: () -> Result<T1, E>,
    producer2: () -> Result<T2, E>,
    producer3: () -> Result<T3, E>,
    producer4: () -> Result<T4, E>,
    producer5: () -> Result<T5, E>,
    producer6: () -> Result<T6, E>,
    producer7: () -> Result<T7, E>,
    producer8: () -> Result<T8, E>,
    producer9: () -> Result<T9, E>,
    producer10: () -> Result<T10, E>,
    transform: (T1, T2, T3, T4, T5, T6, T7, T8, T9, T10) -> Z,
): Result<Z, Collection<E>> {
    contract {
        callsInPlace(producer1, InvocationKind.EXACTLY_ONCE)
        callsInPlace(producer2, InvocationKind.AT_MOST_ONCE)
        callsInPlace(producer3, InvocationKind.AT_MOST_ONCE)
        callsInPlace(producer4, InvocationKind.AT_MOST_ONCE)
        callsInPlace(producer5, InvocationKind.AT_MOST_ONCE)
        callsInPlace(producer6, InvocationKind.AT_MOST_ONCE)
        callsInPlace(producer7, InvocationKind.AT_MOST_ONCE)
        callsInPlace(producer8, InvocationKind.AT_MOST_ONCE)
        callsInPlace(producer9, InvocationKind.AT_MOST_ONCE)
        callsInPlace(producer10, InvocationKind.AT_MOST_ONCE)
        callsInPlace(transform, InvocationKind.AT_MOST_ONCE)
    }

    val result1 = producer1()
    val result2 = producer2()
    val result3 = producer3()
    val result4 = producer4()
    val result5 = producer5()
    val result6 = producer6()
    val result7 = producer7()
    val result8 = producer8()
    val result9 = producer9()
    val result10 = producer10()

    return if (
        result1 is Ok &&
        result2 is Ok &&
        result3 is Ok &&
        result4 is Ok &&
        result5 is Ok &&
        result6 is Ok &&
        result7 is Ok &&
        result8 is Ok &&
        result9 is Ok &&
        result10 is Ok
    ) {
        Ok(
            transform(
                result1.value, result2.value, result3.value,
                result4.value, result5.value, result6.value, 
                result7.value, result8.value, result9.value,
                result10.value,
            ),
        )
    } else {
        Err(
            listOf(
                result1, result2, result3, result4,
                result5, result6, result7, result8,
                result9, result10,
            ).mapNotNull { (it as? Err)?.error },
        )
    }
}

public inline fun <T1, T2, E, Z> zipOrAccumulate(
    producer1: () -> Result<T1, E>,
    producer2: () -> Result<T2, E>,
    transform: (T1, T2, T3) -> Z,
): Result<Z, Collection<E>> { ... }

public inline fun <T1, T2, T3, E, Z> zipOrAccumulate(
    producer1: () -> Result<T1, E>,
    producer2: () -> Result<T2, E>,
    producer3: () -> Result<T3, E>,
    transform: (T1, T2, T3) -> Z,
): Result<Z, Collection<E>> { ... }

public inline fun <T1, T2, T3, T4, E, Z> zipOrAccumulate(...
michaelbull commented 1 year ago

I'd be willing to accept a PR for this.

Interestingly whilst looking at your example code I have thought up some other collection-related functions we might want.

    return if (
        result1 is Ok &&
        result2 is Ok &&
        result3 is Ok &&
        result4 is Ok &&
        result5 is Ok &&
        result6 is Ok &&
        result7 is Ok &&
        result8 is Ok &&
        result9 is Ok &&
        result10 is Ok
    ) {

Here I would extract out all of these into a list of the results, and then have one function call to check that all the items in the list are Ok. Now that I think about it, we can actually offer these as extension functions, something like Collection<E>.allOk(): Boolean and the same for the Error side.

We might also want to leverage the partition function in the implementation of zipOrAccumulate.

I also wonder if there's an example of this in another functional language, Rust/Scala etc? I feel like if there is, they also might have a more succinct name for such a function.

YuitoSato commented 1 year ago

@michaelbull

Thank you for your thoughts.

Indeed, the standard libraries of Scala, Elm, and Rust do not include a direct counterpart to the zipOrAccumulate function. However, the Scala Cats library provides a similar concept with its Validated type.

Validated allows accumulating multiple errors rather than stopping at the first one, similar to what zipOrAccumulate aims to achieve. Furthermore, Cats provides the mapN function which is somewhat similar to zipOrAccumulate in its behavior. The mapN function allows you to combine multiple Validated instances and apply a function to their contained values. Here's a simple example of how mapN might be used:

import cats.data.Validated
import cats.implicits._

val v1: Validated[List[String], Int] = Validated.valid(1)
val v2: Validated[List[String], Int] = Validated.valid(2)

val sum: Validated[List[String], Int] = (v1, v2).mapN(_ + _)

In this example, if both v1 and v2 are Valid, the mapN function will add their values together. If either of them is Invalid, mapN will accumulate the errors.

See also: https://typelevel.org/cats/datatypes/validated#an-iteration-with-validated

However, translating this to Kotlin isn't entirely straightforward. While Scala's mapN can leverage the power of Tuples, which allow you to apply a function to a dynamic number of arguments, Kotlin does not have a similar concept. Considering these points, I think that even though zipOrAccumulate is a somewhat unique feature named in Arrow, it could be seen as a good method design.

If the philosophy of this library is to only implement features that exist in the standard libraries of Scala, Elm, and Rust, I fully respect that. It's always important to maintain consistency with the project's principles and design goals.

I also agree with the introduction of a new allOk method and the use of the partition method for implementation.

michaelbull commented 1 year ago

I think going full-in on a Validated type (or extensions for Result to operate as a Validated type) would probably be a big undertaking. I'm sure it could leverage a lot of the library, just operating in a non-shortcircuiting manner, but would probably take a while to think through properly. This is also something I'm open to.

I'm not intentionally aligning the library with any specific implementation, but looking to existing implementations for inspiration on things like naming and signatures. For example Scala gave us the name for the merge function.

In the short term, I think the allOk & allErrors methods on an Iterable of Result would be great additions to the library if that was something you wanted to tackle before taking on the bigger challenges we've discussed so far.

YuitoSato commented 1 year ago

I understand your thoughts.

So to conclude, should I proceed with the implementation of zipOrAccumulate? Either way, I plan to implement allOk function. Or for this PR, should I implement only smaller methods such as allOk, allErrors, etc?

michaelbull commented 1 year ago

I think the allOk, noneOk, allError, noneError functions can be their own change. I'm happy for you to also continue with zipOrAccumulate as discussed, but it should probably but it's own separate change 👍

YuitoSato commented 1 year ago

Thank you. I'll try to implement zipOrAccumulate this weekend.

YuitoSato commented 9 months ago

@michaelbull I don't want to rush you, but if you have time, could you take a look at the following Pull request? If it doesn't seem to meet your intent, I'll close it. https://github.com/michaelbull/kotlin-result/pull/90

michaelbull commented 9 months ago

@YuitoSato apologies for leaving this one for a while, I've been very busy and unable to commit time to maintenance. I am happy to add it and will try to merge it some time this week and make a new release. I'll update the dependencies at the same time.

michaelbull commented 9 months ago

Hi @YuitoSato. I've merged your change in 27f0a63847a0522686a67bc3672b4c4b73f4c449, however I cut down on the amount of args we support from 10 down to 5. My reasoning for this is that it matches the existing zip we provide - up to 5 producers. I think that anything more than 5 is becoming a bit too unwieldy and probably suggests that the caller should try a different approach, such as maintaining their own list of Results and calling .partition on that list to gather the Ok and Err types. I do understand the use case for validating a bunch of producers otherwise, but think that we should not encourage people to do more than 5 and instead push them towards cleaner solutions.

Please can you have a go with the latest SNAPSHOT version and see if this is too prohibitive? If so we can revisit, but I am trying to avoid bloat of the library and would like to keep the number of functions lower if possible, unfortunately Kotlin requires us to declare a function per number of args.

YuitoSato commented 9 months ago

@michaelbull Thank you for your work and response. I'd like to express my opinion that having support for up to 10 arguments would be beneficial. There are quite a number of classes with six or more fields, and limiting the function to only five arguments might require creating temporary structures. This could lead to cumbersome coding practices. I understand the concern about library bloat, but having the flexibility to handle a reasonable number of arguments, such as 10, would be more convenient in certain situations.

For example, zipOrAccumulate of Arrow.kt Either supports 10 arguments. https://github.com/arrow-kt/arrow/blob/cb033637ab6b6f1f134b1924c948d6638cc5acf4/arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Either.kt#L1499

I appreciate your consideration of this perspective. I've actually used it and it works good with less than 5 arguments. Thank you!

YuitoSato commented 9 months ago

the caller should try a different approach, such as maintaining their own list of Results and calling .partition on that list to gather the Ok and Err types.

In Kotlin, it's not supported to handle individual types for each element of a list, unlike TypeScript. Therefore, I think it's difficult to address this in the partition function. If you want to validate the values of some input, you may need to define a temporary structure, as shown in the example below:

data class Hoge(
    val param1: ParamString,
    val param2: ParamInt,
    val param3: ParamString,
    val param4: ParamInt,
    val param5: ParamString,
    val param6: ParamInt,
    val param7: ParamString,
    val param8: ParamInt,
)

data class Param1To4(
    val param1: String,
    val param2: Int,
    val param3: String,
    val param4: Int,
)

data class ValidatedParam1To4(
    val param1: ParamString,
    val param2: ParamInt,
    val param3: ParamString,
    val param4: ParamInt,
)

data class Param5To8(
    val param5: String,
    val param6: Int,
    val param7: String,
    val param8: Int,
)

data class ValidatedParam5To8(
    val param5: ParamString,
    val param6: ParamInt,
    val param7: ParamString,
    val param8: ParamInt,
)
fun validateStr(string: String): Result<ParamString, Exception> {
    return if (string.isEmpty()) {
        Err(Exception("string is empty"))
    } else {
        Ok(ParamString(string))
    }
}

fun validateInt(int: Int): Result<ParamInt, Exception> {
    return if (int < 0) {
        Err(Exception("int is negative"))
    } else {
        Ok(ParamInt(int))
    }
}

fun validateParam1To4(param1To4: Param1To4): Result<ValidatedParam1To4, Collection<Exception>> {
    val (param1, param2, param3, param4) = param1To4

    return zipOrAccumulate(
        { validateStr(param1) },
        { validateInt(param2) },
        { validateStr(param3) },
        { validateInt(param4) },
    ) { param1, param2, param3, param4 ->
        ValidatedParam1To4(
            param1 = param1,
            param2 = param2,
            param3 = param3,
            param4 = param4,
        )
    }
}

fun validateParam5To8(param5To8: Param5To8): Result<ValidatedParam5To8, Collection<Exception>> {
    val (param5, param6, param7, param8) = param5To8

    return zipOrAccumulate(
        { validateStr(param5) },
        { validateInt(param6) },
        { validateStr(param7) },
        { validateInt(param8) },
    ) { param5, param6, param7, param8 ->
        ValidatedParam5To8(
            param5 = param5,
            param6 = param6,
            param7 = param7,
            param8 = param8,
        )
    }
}

fun validateAndTransform(
    param1: String,
    param2: Int,
    param3: String,
    param4: Int,
    param5: String,
    param6: Int,
    param7: String,
    param8: Int,
): Result<Hoge, Collection<Exception>> {
    val param1To4 = Param1To4(
        param1 = param1,
        param2 = param2,
        param3 = param3,
        param4 = param4,
    )

    val param5To8 = Param5To8(
        param5 = param5,
        param6 = param6,
        param7 = param7,
        param8 = param8,
    )

    return zipOrAccumulate(
        { validateParam1To4(param1To4) },
        { validateParam5To8(param5To8) },
    ) { param1To4, param5To8 ->
        Hoge(
            param1 = param1To4.param1,
            param2 = param1To4.param2,
            param3 = param1To4.param3,
            param4 = param1To4.param4,
            param5 = param5To8.param5,
            param6 = param5To8.param6,
            param7 = param5To8.param7,
            param8 = param5To8.param8,
        )
    }.mapError { it.flatten() }
}
YuitoSato commented 9 months ago

Also, some of zipOrAccumulate methods returns Result<V, Collection<E>> and some returns Result<V, List<E>>. Maybe mistakes?

public inline fun <T1, T2, T3, E, V> zipOrAccumulate(
    producer1: () -> Result<T1, E>,
    producer2: () -> Result<T2, E>,
    producer3: () -> Result<T3, E>,
    transform: (T1, T2, T3) -> V,
): Result<V, List<E>> { ...}

public inline fun <T1, T2, T3, T4, T5, E, V> zipOrAccumulate(
    producer1: () -> Result<T1, E>,
    producer2: () -> Result<T2, E>,
    producer3: () -> Result<T3, E>,
    producer4: () -> Result<T4, E>,
    producer5: () -> Result<T5, E>,
    transform: (T1, T2, T3, T4, T5) -> V,
): Result<V, Collection<E>>  {...}
YuitoSato commented 9 months ago

I agree with using Collection, but in that case it might be better to use Generics.

public inline fun <T1, T2, T3, T4, E, V, C : Collection<V>> zipOrAccumulate(
    producer1: () -> Result<T1, E>,
    producer2: () -> Result<T2, E>,
    producer3: () -> Result<T3, E>,
    producer4: () -> Result<T4, E>,
    transform: (T1, T2, T3, T4) -> V,
): Result<V, C> {}