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

Accumulate DSL for Raise (open for discussion) #2881

Closed
wants to merge 2 commits into from

Conversation

serras
Copy link
Member

@serras serras commented Dec 30, 2022

This creates a "sub-DSL" within Raise to express scopes in which errors should be accumulated, instead of the regular fail-first approach. The idea is that inside accumulateErrors you "protect" each potentially-failing computation with accumulate.

data class Name(val first: String, val last: String)

fun Raise<String>.validFirstName(s: String): String = TODO()
fun Raise<String>.validLastName(s: String): String = TODO()

fun Raise<NonEmptyList<String>>.validName(
  first: String, last: String
): Name = accumulateErrors {
  val f = accumulate { validFirstname(first) }
  val l = accumulate { validLastName(last) }
  Name(f.value, l.value)
}

The problem here is potential data dependencies. To fight these, the result of accumulate is not directly A, but Accumulate.Value<A>, a "box" which remembers whether the computation it comes from failed or not. If at some point we try to unwrap a failed computation, that means we cannot proceed further with accumulation, and we just return whatever errors we had found.

As a comparison with other FP abstractions, Accumulate implements selective functors, because it's allowed to choose how to continue based on whether a computation failed or not, but not based on the data you obtained (that requires monads).

@serras serras requested review from pakoito, nomisRev and a team December 30, 2022 14:48
@serras
Copy link
Member Author

serras commented Dec 30, 2022

By the way, I tried to make this work without the intermediate accumulate, but the fact that raise returns Nothing is a blocker, since here we would need it to return Accumulate.Value instead.

@github-actions
Copy link
Contributor

@pakoito
Copy link
Member

pakoito commented Jan 2, 2023

Isn't this just zip by another name? It feels like it breaks sequentiality, which isn't intuitive.

@nomisRev
Copy link
Member

nomisRev commented Jan 2, 2023

Interesting 🤔 This looks very similar to select from KotlinX Coroutines which I guess implements selective functors for IO.

else -> raise(EmptyValue.unbox(e))
): List<B> =
accumulateErrors(semigroup) {
map { accumulate { block(it) } }.map { it.value }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunate, this introduces a second iteration over the Iterable.

@nomisRev
Copy link
Member

nomisRev commented Jan 2, 2023

DRAFT?

@serras serras marked this pull request as draft January 2, 2023 08:59
@serras
Copy link
Member Author

serras commented Jan 31, 2023

I'm closing this in favor of #2880. I think this is just too complicated as an API to expose.

@kyay10
Copy link
Collaborator

kyay10 commented Jan 19, 2024

When/if we have union types, this idea could be great since, it seems, the "idiomatic" way to represent errors would be returning such a union. Smart casts could even make this nicer, with a simple ensureAllSucceeded(...) that has contracts to smart case all the values to be without our failure object. You can already see this structure presenting itself in how zipOrAccumulate is implemented. Perhaps this can be an additional DSL to it, to allow a different style.

@nomisRev
Copy link
Member

@kyay10 this was proposed by @Intex32, but it's currently not working properly with contracts yet 😞
I'm hoping this will be fixed in K2, but currently contracts behave weirdly in some scenarios if used cross module.

I.e. if(Either.isRight()) should allow you to access .value but it currently doesn't work from outside of Arrow.

Union would be amazing to have, and would allow us to spice up a lot of the APIs. context(Raise<A | B>) 😍 🔥

I think we should really revisit this when we have those features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants