elm-script icon indicating copy to clipboard operation
elm-script copied to clipboard

Collect-like function that doesn't give up on errors

Open MartinSStewart opened this issue 4 years ago • 6 comments

It would be useful to have a function that behaves like collect except that it doesn't stop when it encounters an error. Instead it always runs for every item and if there are any errors, it fails with a list of those errors, otherwise it succeeds with a list of values.

separate : List (Result e a) -> ( List e, List a )
separate =
    List.foldl
        (\result ( errors, oks ) ->
            case result of
                Ok ok ->
                    ( errors, ok :: oks )

                Err error ->
                    ( error :: errors, oks )
        )
        ( [], [] )


collectAll : (a -> Script x b) -> List a -> Script (List x) (List b)
collectAll getScript =
    Script.collect (getScript >> Script.attempt)
        >> Script.thenWith
            (\results ->
                case separate results of
                    ( [], oks ) ->
                        Script.succeed oks

                    ( errors, _ ) ->
                        Script.fail errors
            )

One advantage of using this over collect is that you can provide better error messages. Rather than having the user having to fix each problem they encounter one at a time, they can be presented with a list of everything that went wrong.

MartinSStewart avatar Jun 04 '20 13:06 MartinSStewart

I like this idea, but I'd be inclined to modify it slightly:

collectAll : (a -> Script x b) -> List a -> Script ( x, List x ) (List b)

This way the user never has to consider the possibility that the script might 'fail' with an empty list of errors. And I think the slightly more complex type signature is OK given that there's collect for the simple case.

ianmackenzie avatar Jun 04 '20 16:06 ianmackenzie

I started looking at this and realized that do, each and sequence could all potentially use the same treatment - but following the same pattern, eachAll is a travesty 😛 Any thoughts on a naming pattern that might work for all four functions? One option would just be to suffix each function with _; it looks a bit ugly but I'm pretty OK with how it's turned out with at and at_ in elm-units and it's vaguely similar to the relationship between things like mapM and mapM_ in Haskell.

ianmackenzie avatar Jun 11 '20 23:06 ianmackenzie

Haha, eachAll doesn't exactly roll off the tongue. I think adding All is better than _ though. To me, _ makes more sense when a function is identical to another except the argument order has been changed.

MartinSStewart avatar Jun 12 '20 06:06 MartinSStewart

Here's another possibility I just thought of (open to alternate names for ExecutionPolicy and its various implementations):

-- Opaque type that defines how to run a list of scripts and collect errors/results
type ExecutionPolicy x a y b
    = ExecutionPolicy (List (Script x a) -> Script y b)

-- Either fail with the first error (and stop there), or succeed with all results
stopOnFirstError : ExecutionPolicy x a x (List a)

-- Don't stop on error; either report all errors, or succeed with all results
reportAllErrors : ExecutionPolicy x a ( x, List x ) (List a)

-- Wrap each script in 'attempt' and simply return a list of Results
attemptAll : ExecutionPolicy x a Never (List (Result x a))

-- could come up with other kinds of execution policies, or potentially
-- allow users to construct their own custom ones

sequence : List (Script x a) -> Script x (List a)
sequence =
    sequenceWith { executionPolicy = stopOnFirstError }

sequenceWith : 
    { executionPolicy : ExecutionPolicy x a y b } 
    -> List (Script x a) 
    -> Script y b
sequenceWith { executionPolicy } scripts =
    let
        (ExecutionPolicy policy) =
            executionPolicy
    in
    policy scripts

do : List (Script x ()) -> Script x ()

-- The whole point of 'do' is to take something that would otherwise result in
-- a script with a 'List ()' return type and collapse that to just '()', so
-- reflect this in the required execution policy type (which means that
-- 'attemptAll' couldn't be used here)
doWith : 
    { executionPolicy : ExecutionPolicy x () y (List ()) }
    -> List (Script x ()) 
    -> Script y ()

each : (a -> Script x ()) -> List a -> Script x ()

-- Same restriction as 'do'
eachWith : 
    { executionPolicy : ExecutionPolicy x () y (List ()) }
    -> (a -> Script x ()) 
    -> List a 
    -> Script y ()

collect : (a -> Script x b) -> List a -> Script x (List b)

collectWith : 
    { executionPolicy : ExecutionPolicy x b y c } 
    -> (a -> Script x b) 
    -> List a 
    -> Script y c

Then you can write Script.collect function values but also

Script.collectWith { executionPolicy = Script.reportAllErrors } function values

ianmackenzie avatar Jun 12 '20 14:06 ianmackenzie

That's an interesting idea! I guess if it's worth using comes down to how often the user needs to use different kinds of execution policies. If they only want the fail immediately version for example, then this might just be unnecessary mental overhead. I think it's worth trying this out though.

MartinSStewart avatar Jun 12 '20 18:06 MartinSStewart

Yeah, I definitely think that the "fail immediately" version should be the default (the non-*With version) so you don't have to worry about execution policies at all in the simple case. And I can do things with the docs to avoid clutter/too much mental overhead, like moving all the *With functions to the bottom in an 'Advanced' section or similar, but link to them from the simple versions with a note like "If you want to collect all errors instead of just the first...".

ianmackenzie avatar Jun 12 '20 18:06 ianmackenzie