serokell / universum

:milky_way: Prelude written in @Serokell
MIT License
176 stars 28 forks source link

Change arguments order for foldl' #91

Closed chshersh closed 6 years ago

chshersh commented 6 years ago

Currently foldl' function has next type:

foldl' :: forall a b . (b -> a -> b) -> b -> [a] -> b

This type signature is not very convenient to use. Consider function where you want to insert elements of list one by one into HashSet (this is trivial example, but in practice something structurally similar appears very often):

foo :: Hashable key => [key] -> HashSet key
foo = foldl' (flip HashSet.insert) mempty

You either add flip or write your own function inside where. Or use foldr, but foldr performance is abysmal and such implementation will have space leak (TODO: benchmark and profile to prove it). It's a common pattern in Haskell where data structure is the last argument of function. Because it's convenient to use so. But when you want to use this pattern with foldl' you can't rely on eta-reduce anymore :( You can't just write

foo :: Hashable key => [key] -> HashSet key
foo = foldl' HashSet.insert mempty

So I propose to have our foldl' with arguments changed:

foldl' :: forall a b . (a -> b -> b) -> b -> [a] -> b

This should be implemented using foldl' from base. And some benchmarks should be added as well. I checked GHC.List module and didn't notice presence of foldl' inside RULES pragmas. So this change can be performed without performance penalties.

DRAWBACKS

  1. Unexpected arguments order for users of foldl' from base (shouldn't be very important because not many people uses foldl')
  2. Potential performance penalties if we implement foldl' in a wrong way.

ADVANTAGES

  1. More convenient library API.
chshersh commented 6 years ago

Done in PR #106.

flyingleafe commented 6 years ago

Wow, this is actually an API-breaking change which was done via incrementing patch version instead of major version (!). Why would you ever do that? Also, it's super weird when foldl and foldl' have different type signatures, it's completely counter-intuitive.

(Got frustrated recently after a failed attempt to build several packages from cardano-sl-* family with new Universum)

chshersh commented 6 years ago

@flyingleafe version change was already discussed.

it's super weird when foldl and foldl'

You shouldn't use foldl anyways. And universum shouldn't export foldl if it does this now. So it doesn't matter at the end.

(Got frustrated recently after a failed attempt to build several packages from cardano-sl-* family with new Universum)

You just need to wait for this PR to be merged: