Handling bill cancellation

Question

I'm trying to refactor the working code below which intends to handle bill cancellation, but doesn't seem "linear" enough.

open FSharpPlus
open FSharpPlus.Data

type BillCancellationReason = | A | B
type OrderBillingCommand = | A | B
type OrderBillingEvent = | A | B
type CommandError = | A | B

type BillCancellationRequest =
    { BillSequence: int32
      Reason: BillCancellationReason
      Comment: string }

type BillCancellationsRequestPart =
    { OrderId: int32
      BillSequence: int32
      Reason: BillCancellationReason
      Comment: string }

type BillCancellationsRequest = BillCancellationsRequestPart list

module Validation =
    // Validation<'Failure list, 'Success> list -> Validation<'Failure list, 'Success list>
    let cumulativeFoldList (validations: Validation<'Failure list, 'Success> list):
        Validation<'Failure list, 'Success list> =
        traverse id validations

// BillCancellationsRequestPart -> Validation<CommandError list, (int32 * OrderBillingCommand)>
let toCommand (requestPart: BillCancellationsRequestPart) =
    Unchecked.defaultof<Validation<CommandError list, (int32 * OrderBillingCommand)>>

// (int32 * OrderBillingCommand) list -> Async<Result<OrderBillingEvent list, CommandError list>>
let processValidCommands (commands: (int32 * OrderBillingCommand) list) =
    Unchecked.defaultof<Async<Result<OrderBillingEvent list, CommandError list>>>

// BillCancellationsRequest -> Async<Result<OrderBillingEvent list,CommandError list>>
let cancelBills (request: BillCancellationsRequest) =
    async {
        match request |> List.map toCommand |> Validation.cumulativeFoldList with
        | Success commands -> return! processValidCommands commands
        | Failure failure -> return Error failure
    }

In particular this last bit:

// BillCancellationsRequest -> Async<Result<OrderBillingEvent list,CommandError list>>
let cancelBills (request: BillCancellationsRequest) =
    async {
        match request |> List.map toCommand |> Validation.cumulativeFoldList with
        | Success commands -> return! processValidCommands commands
        | Failure failure -> return Error failure
    }

I tried to come up with a new module but I found myself rolling a lot more code:

module Result =
    // ('T -> 'U) -> ('V -> 'W) -> Result<'T, 'V> -> Result<'U, 'W>
    let bimap okMapping errorMapping result =
        result
        |> Result.map okMapping
        |> Result.mapError errorMapping

    // Result<Async<'T>, 'U> -> Async<Result<'T, 'U>>
    let awaitOnOk result =
        async {
            match result with
            | Ok ok ->
                let! awaitedOk = ok
                return Ok awaitedOk
            | Error error ->
                return Error error
        }

// BillCancellationsRequest -> Async<Result<OrderBillingEvent list,CommandError list>>
let cancelBills (request: BillCancellationsRequest) =
    request
    |> List.map toCommand
    |> Validation.cumulativeFoldList
    |> Validation.toResult
    |> Result.map processValidCommands
    |> Result.awaitOnOk
    |> Async.map Result.flatten

I'm wondering if there is like a better and more concise approach to achieve that?

Note: Unchecked.defaultof<...> has been used just for the sake of making the code above to compile.

Answer 1

To be honest, your original implementation of cancelBills looks pretty good - it's very focused and is fairly easy to understand what it's doing even without much context.

If you really want to write the function as a single chain of functions then your alternative version is most of the way there. I'd consider replacing the last three functions in the chain with a single Result.asyncBind function to simplify it somewhat, i.e.

module Result  =
    let asyncBind (asyncBinder: 'T -> Async<Result<'U, 'TError>>) (result: Result<'T, 'TError>) =
        match result with
        | Result.Ok value -> asyncBinder value
        | Result.Error error -> async { return Result.Error error }

This is effectively just an async equivalent of the built-in Result.bind, so shouldn't add much overhead when reading the code and should be fairly re-usable if you're piping Result and Async around a lot. That being said, if you're not going to be re-using Result.asyncBind (or your equivalent function chain) then just stick with your original implementation of cancelBills!