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

[email protected] #12

Merged
merged 1 commit into from
Nov 11, 2016
Merged

[email protected] #12

merged 1 commit into from
Nov 11, 2016

Conversation

davidchambers
Copy link
Member

This pull request adds TypeClass values for Alt, Plus, and Alternative, and functions for fantasy-land/alt and fantasy-land/zero, all of which were added in fantasyland/fantasy-land#197.

Please review this when you have some time, @safareli. :)

@safareli
Copy link
Member

safareli commented Nov 6, 2016

If Array is alternative String could be Alternative in the same way too.

list/Array/String we could have other instance, which just does concatenation. In purescript and GHC (search Alternative []) we have that concatenative version so maybe it's better to stay consistent with them?

I could not find implementation of alternative for Map nor in PS nor in Haskell , but If we follow use concatenation as alt for string and array maybe we should do same for StrMap ?

@davidchambers
Copy link
Member Author

Thanks for the feedback, @safareli. I've updated the patch. Could you take another look?

If Array is alternative String could be Alternative in the same way too.

Can it, though? String does not satisfy the requirements of Functor.

@safareli
Copy link
Member

safareli commented Nov 6, 2016

Can it, though? String does not satisfy the requirements of Functor.

oh, yes :d

//. true
//.
//. > Alternative.test({})
//. false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be true

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. We can't define fantasy-land/of for Object, so Object can't satisfy Applicative.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, yes, again

@safareli
Copy link
Member

safareli commented Nov 6, 2016

LGTM

@davidchambers davidchambers merged commit affa2c4 into master Nov 11, 2016
@davidchambers davidchambers deleted the dc-fantasy-land branch November 11, 2016 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants