Proposal: Remove top-level fold in favor of recover et al

[kyay10]

I've been thinking about the need for a transform lambda for top-level fold for raise. I understand that it makes sense for there to be a transform in the happy path if we're thinking of a Raise<E>.() -> A as an Effect, i.e as a proper data type, but I think the mental model surrounding Raise seems to be more around error handlers than it is about representing a data type that can succeed and fail. I think looking at all the raise builders other than fold makes that very clear, because none of them take any transformation parameter for the happy path. That's because a transformation is just a last step of the calculation inside our Raise<E>.() -> A block, so we can always very idiomatically do such a transformation inside the block with no cost. I see how Effect.fold is very idiomatic though, and it makes since for it to have a transform

When removing the transform lambda, it made it visible that perhaps top-level fold shouldn't exist, instead being replaced by recover, which matches the mental model of raise providing error handling strategies. This PR does that, and perhaps one can see how some of Arrow's own code becomes clearer. I understand however that maybe the fold ship has already sailed!

[kyay10]

I dislike that this is needed. You'd expect that all we need is the one-liner:
recover({ transform(invoke()) }, { recover(it) }, { catch(it) })
but sadly this has slightly different behaviour when transform invokes a captured raise call that was captured in block. This is because fold called Raise.complete before it executed the transform, while the one-liner calls transform before Raise.complete, thus resulting in a captured raise that is immediately invoked. This violates the test EffectSpec.shiftLeakedResultsInRaiseLeakException which seems to suggest that this behavior is intentional. Thus, we have to use non-local returns to get rid of all our error cases, and then in our happy path we execute the transform, keeping in mind that it can throw

[serras]

I don’t think we should merge this. Apart from breaking compatibility, this would break the expectation of what a fold is, which should provide a way to apply a function to each of the branches of a type. In the case of Raise, these branches are success, failure, and exception, hence the current type.

[kyay10]

I don’t think we should merge this. Apart from breaking compatibility, this would break the expectation of what a fold is, which should provide a way to apply a function to each of the branches of a type. In the case of Raise, these branches are success, failure, and exception, hence the current type.

My question is though, is Raise often used in such a way? I understand (Eager)Effect being used as such, but for Raise, I think its usage tends to be for error-recovery, which is much better served by recover.

It's likely a matter of personal taste, but when looking at the places in Arrow's codebase that changed from this PR, most places got clearer in their intent to handle errors. In fact, they look a lot more like what an imperative try-catch version of them would be like, with clear focus on the happy path, then error handling afterwards.

Take for example this change:

fold<NonEmptyList<Error>, B, Unit>(
  { transform(RaiseAccumulate(this), item) },
  { errors -> error.addAll(errors) },
  { results.add(it) }
)

to

recover(
  { results.add(transform(RaiseAccumulate(this), item)) },
  { errors -> error.addAll(errors) }
)

The former separates the operation from the transformation. It's rather unnatural considering that the operation is a lambda right there that can be modified to include the transformation easily. The latter does the operation and transformation in one go, and only afterwards does it handle errors.
Also, if our transformation gets sufficiently complex, it might want to raise errors, too. It would then have to be moved to the operation regardless, and we'd end up with a recover.
I think this creates an unnecessary separation between the operation that can fail, and a transformation afterwards that must succeed.

For Effect, however, fold makes perfect sense. Our operation has already been built up previously, so it's less of an operation and more of a data type whose values are evaluated lazily.

I think that the rest of the Raise-related operations use a mental model of imperative code with typed error handling, and hence fold is a weird corner where that mental model isn't utilised.
Of course, this is merely just a suggestion. I was looking through Arrow's codebase and found that Raise fold usages took more mental effort for me to understand than those involving other builders.

[serras]

After some discussion, we've decided that changing fold, a fundamental building block for Raise is a very risky move.

• You can describe computations in which the final transformation is done either in the same context (by composing the function in the block itself) or outside of it (by using transform). These two options are, by design, not the same.
• Functions like merge are very difficult to express without it. The fact that you do not require the block and the error type to be equal in fold, but rather turn them into a similar type afterwards gives you additional power (think on the implementations of either or result).

Whereas fold is maybe something to be used sparsely (maybe we should make it less obvious in the docs), we are not changing that part of the API for now.

(@kyay10 thanks for raising this issue, making concrete how such a reinterpretation of the API would look like)