emmanueltouzery / prelude-ts

Functional programming, immutable collections and FP constructs for typescript and javascript
ISC License
377 stars 21 forks source link

Validation - accumulating errors with Either #61

Open flakey-bit opened 2 years ago

flakey-bit commented 2 years ago

I've got some code like this

// We're populating a FinancialTransaction bit by bit

// Start in the "right" state with just the transactionId populated
const initial = Either.right<TransactionParseFailure, Partial<FinancialTransaction>>({ transactionId });

// Curry the raw-data in - populateAmount takes a partial financial transaction and returns either 
// - a TransactionParseFailure (left) or 
// - a partial financial transaction with the amount field populated (right)
const populateAmount = PopulateAmount(rawData);

// Curry the raw-data in - populateTransactionDate takes a partial financial transaction and returns either 
// - a TransactionParseFailure (left) or 
// - a partial financial transaction with the date field populated (right)
const populateTransactionDate = PopulateTransactionDate(rawData);

return initial
  .flatMap(populateAmount)
  .flatMap(populateTransactionDate)
  .map(PopulateOptionalFields);

However I'd like to change this so that we accumulate the errors - at the moment, it stops as soon as it ends in a left state. I think normally I'd do this with a Validation type, however prelude-ts is missing one currently.

I can't seem to get my head around how to achieve this with Either.sequenceAcc or Either.liftApAcc

emmanueltouzery commented 2 years ago

Yes, sequenceAcc is meant to transform a list of either values into an either of a list. and liftApAcc is meant to enable to take a function that doesn't operate on Either values, and "lift" it so it can operate on either values.

So none of them enables exactly what you want.

However I feel that flatMap for a validation/either accumulation purpose is a bit artificial, because the point is that you're supposed to transform the value, meaning often change the type of the value you work on.

so, if i have: fnA: T -> Either<Error, U> fnB: U -> Either<Error, V>

first you call fnA, and it returns Left. How are we going to call fnB? We don't have a U to give to it. We do have the T, but not any U. So we'd be forced to short-circuit at this point, unlike what would be intended.

If I look at the implementation of Validation.flatMap from the vavr java library, the contents support this:

    @SuppressWarnings("unchecked")
    public final <U> Validation<E, U> flatMap(Function<? super T, ? extends Validation<E, ? extends U>> mapper) {
        Objects.requireNonNull(mapper, "mapper is null");
        return isInvalid() ? (Validation<E, U>) this : (Validation<E, U>) mapper.apply(get());
    }

in case the validation is invalid, it returns this and casts it as the other type..

So I feel flatMap would only work if we keep the same type T all along. Which is a special case more than the general case.

I realize that you most likely gave here a very simplified sample, but here's how I would do it:

function populateTransaction({initial: Initial, amount: number, date: Date}) {
 return new Transaction(initial, amount, date);
}

// {initial: Either<E, Initial>, amount: Either<E, number>, date: Either<E, Date>} => Either<Vector<E>, Transaction>
const populateEitherTx = Either.liftApAcc(populateTransaction);

const amount = initial.map(computeAmount);
const date = initial.map(computeDate);

return populateEitherTx({initial, amount, date});
flakey-bit commented 2 years ago

However I feel that flatMap for a validation/either accumulation purpose is a bit artificial, because the point is that you're supposed to transform the value, meaning often change the type of the value you work on.

Yeah, I think you're right there. Indeed even in my case the types are changing - e.g. we go from

Partial<FinancialTransaction> & Pick<FinancialTransaction, "transactionId">

to

Partial<FinancialTransaction> & Pick<FinancialTransaction, "transactionId" | "amount">

and then likewise we add on the date.

Thanks for taking the time to respond & your suggestion, I'll give it a go. Feel free to close this issue 💚