Remove fold1 from Foldable1
kl0tl opened this issue · 6 comments
Foldable has no fold member and Data.Foldable defines fold = foldMap identity but Foldable1 has a fold1 member and Data.Semigroup.Foldable defines fold1Default = foldMap1 identity. Since the addition of foldr1 and foldl1 is already a breaking change, should we also remove fold1 from Foldable1 and rename fold1Default to fold1 for consistency?
Seems sensible to me, though this would force us to update already-updated libraries.
I can update the already updated libraries if this is something we want to do, that would only be fair.
I'm uneasy on this. I'm not sure consistency on its own is a strong enough reason to make a breaking change, and I think we need to consider more carefully why it ended up the way it did.
I think we need to consider more carefully why it ended up the way it did.
So... run a git blame and see what PRs surround the fold1 situation?
I suspect our Foldable1 has a fold1 method because so does the Haskell inspiration. But Foldable has a fold method too in Haskell! I guess classes tend to have more derivable methods in Haskell because they’re cheap when given default implementations.
Do we really need the potential performance improvement brought by a specialized implementation which would just avoid the Category dictionary creation and the identity function call, when this promote an inefficient default implementation of foldMap1? Granted we’re not there yet, but wouldn’t an inliner achieve that for the default implementation anyway?
Ok, yes, since we already have foldMap1, foldl1 and foldr1 in the class and since we're already making a breaking change (by adding foldr1 and foldl1) which requires all existing instances to be updated, I agree that there's not really a need to keep fold1 in the class. I certainly don't think the performance cost of replacing the implementation of fold1 with foldMap1 identity is important enough to justify keeping it in the class.