Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Result.Extra.combineFoldl #60

Open
miniBill opened this issue Jul 23, 2024 · 8 comments
Open

Add Result.Extra.combineFoldl #60

miniBill opened this issue Jul 23, 2024 · 8 comments
Labels
new function Request to add a new function to the library

Comments

@miniBill
Copy link
Collaborator

combineFoldl

combineFoldl :
    (a -> acc -> Result err acc)
    -> acc
    -> List a
    -> Result err acc
combineFoldl f acc list =
    case list of
        [] ->
            Ok acc

        head :: tail ->
            case f head acc of
                Err e ->
                    Err e

                Ok v ->
                    resultFoldl f v tail

Motivating use case

It's common to fold over a list while keeping track of some state. Doing Result.andThen / Result.map is a List.foldl does not short circuit, whereas this function does.

@miniBill miniBill added the new function Request to add a new function to the library label Jul 23, 2024
@miniBill
Copy link
Collaborator Author

Why not combineFoldr too?

Because it's much harder to implement because you need to traverse the whole list, and at that point might as well use List.foldr

@miniBill
Copy link
Collaborator Author

@gampleman opinions?

@gampleman
Copy link
Collaborator

  1. I'd like to see some comparison vs using List.Extra.stoppableFoldl, since that aims at more or less the same use case.

  2. I feel like we should figure out some consistent policy for these sorts of functions if they go into List.Extra or Result/Maybe.Extra. My gut feeling here is that this is more of a List.Extra thing, but can't really articulate a coherent argument either way.

  3. Can you explain your rationale for the naming? I'm not really seeing much of a commonality between this and the other Result.Extra.combine* functions...

@miniBill
Copy link
Collaborator Author

miniBill commented Jul 25, 2024

  1. It's very similar, but step is Step a whereas result is Result e x, and the additional type parameter is essential
  2. I'd say this goes in Result.Extra, and an hypothetical combineFoldl with Maybe goes into Maybe.Extra
  3. Terrible name, please help come up with a better one

@gampleman
Copy link
Collaborator

It's very similar, but step is Step a whereas result is Result e x, and the additional type parameter is essential

Yeah but Step (Result e x) is what you could quite easily do. So I guess the question is: is it worth adding this extra abstraction? I think having some code examples written with both would be helpful.

@miniBill
Copy link
Collaborator Author

miniBill commented Aug 12, 2024

withCombineFoldl : List String -> Result String (Dict String Int)
withCombineFoldl list =
    -- Result.Extra.combineFoldl
    combineFoldl
        (\item acc ->
            case parse item of
                Err e ->
                    Err e

                Ok ( k, v ) ->
                    Ok (Dict.insert k v acc)
        )
        Dict.empty
        list


withStoppableFoldl : List String -> Result String (Dict String Int)
withStoppableFoldl list =
    List.Extra.stoppableFoldl
        (\item acc ->
            case acc of
                Err e ->
                    -- This never happens
                    List.Extra.Stop (Err e)

                Ok okAcc ->
                    case parse item of
                        Err e ->
                            List.Extra.Stop (Err e)

                        Ok ( k, v ) ->
                            List.Extra.Continue (Ok (Dict.insert k v okAcc))
        )
        (Ok Dict.empty)
        list

It's not terrible to be honest. But it's also not great.

@gampleman
Copy link
Collaborator

And perhaps like this, which is fairly nice

withCombineFoldl : List String -> Result String (Dict String Int)
withCombineFoldl =
    Result.Extra.combineFoldl
        (\item acc ->
            parse item
                |> Result.map (\( k, v ) -> Dict.insert k v acc)
        )
        Dict.empty

hm OK, I think I'm coming around to this. I'm still not in love with the name. What about foldlUntilError or some such?

@miniBill
Copy link
Collaborator Author

That's not a terrible name!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new function Request to add a new function to the library
Projects
None yet
Development

No branches or pull requests

2 participants