4
\$\begingroup\$

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.

\$\endgroup\$
9
  • 1
    \$\begingroup\$ @BCdotWEB don't you think that "nested" is making the question specific enough? Not criticizing what you're saying, I just found my question pretty clear and well-scoped. \$\endgroup\$ – Kerry Perret Nov 7 '20 at 20:18
  • 3
    \$\begingroup\$ Your question is clear and well-scoped, but your title should only state the task accomplished by your code. \$\endgroup\$ – Aryan Parekh Nov 7 '20 at 20:28
  • 1
    \$\begingroup\$ @AryanParekh "ahhhh" alright sorry, didn't get that right, so in my case saying something like "F#: handling bills cancellation" is okay? \$\endgroup\$ – Kerry Perret Nov 7 '20 at 22:29
  • 1
    \$\begingroup\$ Yes, whatever you think best titles your code :) \$\endgroup\$ – Aryan Parekh Nov 7 '20 at 22:45
  • 1
    \$\begingroup\$ @AryanParekh alrighty, hope this is good enough. \$\endgroup\$ – Kerry Perret Nov 8 '20 at 1:39
1
\$\begingroup\$

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!

\$\endgroup\$
1
  • \$\begingroup\$ thanks Rob for your input! \$\endgroup\$ – Kerry Perret Dec 11 '20 at 17:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.