Skip to content
This repository has been archived by the owner on Oct 4, 2020. It is now read-only.

Add fold, foldMap, foldM, foldMaybe #115

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

natefaubion
Copy link
Contributor

No description provided.

@hdgarrood
Copy link
Contributor

I wonder if the new FoldableWithIndex type class might be relevant here? https://pursuit.purescript.org/packages/purescript-foldable-traversable/3.4.0/docs/Data.FoldableWithIndex

@natefaubion
Copy link
Contributor Author

I had no idea this class existed 😆

@garyb
Copy link
Member

garyb commented Jul 13, 2017

But yeah, adding WithIndex stuff for both Map and StrMap sounds good too. I think we can do that?

@garyb
Copy link
Member

garyb commented Jul 13, 2017

Of course we can, Map is a Functor.

@paf31
Copy link
Contributor

paf31 commented Jul 13, 2017

Will these go into instances then? If not, can we use the xWithKey naming scheme?

@natefaubion
Copy link
Contributor Author

I was gunning for API parity with StrMap. I'm happy to make modifications if we want to shift over to instances, but we should do the same for StrMap, and it will be a breaking change (or we can add deprecated warnings).

@paf31
Copy link
Contributor

paf31 commented Jul 13, 2017

I agree they should ideally match. Maybe let's just add the instances on top of what you have now, and we can remove them later.

@hdgarrood
Copy link
Contributor

I'm not sure if it is necessarily a problem to provide monomorphic functions as well as an instance providing those same functions in general - sometimes the former are more convenient, I think.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants