search

elm-community / list-extra

Convenience functions for working with List.
http://package.elm-lang.org/packages/elm-community/list-extra/latest
MIT License
135 stars 58 forks source link

Fix behavior of foldl1 #92

Closed pzp1997 closed 6 years ago

pzp1997 commented 6 years ago

The behavior of foldl1 is not in line with the behavior of List.foldl. The problem is that the accumulator for List.foldl is the second argument of the "combine" function while for foldl1, it is the first argument. The cause of this mistake can be traced most likely to Haskell, where the type of foldl is (b -> a -> b) -> b -> List a -> b, i.e. the combine function is implicitly flipped.

I have corrected the behavior which incidentally also lead to faster implementations for foldl1 and foldr1. I also wrote better examples for both functions that more clearly demonstrate the difference between them.

Note that the changes that I made are breaking despite the type of foldl1 being unchanged.

Chadtech commented 6 years ago

Great catch @pzp1997 ! Thanks for the PR. Also, could you share a benchmark link?

pzp1997 commented 6 years ago

Here is the foldl1 benchmark and the foldr1 benchmark.

Chadtech commented 6 years ago

The benchmarks look good to me! Thanks for the contribution @pzp1997 . Since this is a breaking change, I will merge this on our next major release.

I wonder if its worth updating the future docs to mention this breaking change? Heres a PR I just made to include an explanation of the current behavior in our current docs #94