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

Add <^> and curried map() #86

Merged
merged 8 commits into from
May 8, 2015
Merged

Add <^> and curried map() #86

merged 8 commits into from
May 8, 2015

Conversation

sharplet
Copy link
Collaborator

Fixes #83.

I was mostly happy with this until I realised that this is the same thing as -->, and now there's three ways to do the same thing! As a result there's now both Reduction.swift and Map.swift, which seems a bit confusing given that --> is also map.

¯_(ツ)_/¯

@robrix
Copy link
Owner

robrix commented Apr 28, 2015

Hm, I’d forgotten --> is literally map. I’m used to calling it “reduction” since that’s the term applied in the derivative parsing paper.

I guess we could:

  • have multiple operators
  • replace --> with <^>
  • leave --> in peace

I’m not fond of the first one, and am unsure about the other two. Thoughts?

@neilpa
Copy link
Contributor

neilpa commented Apr 28, 2015

There's a fourth option -- delete just the parser-only overload of --> which resolves #80. Then there's distinction between the reduction and mapping operators.

@robrix
Copy link
Owner

robrix commented Apr 28, 2015

Good point, @neilpa.

@sharplet
Copy link
Collaborator Author

I think the --> operator is nice from a public API point of view. It has an interesting finality to it, that makes it feel natural for building your model objects. But it feels different to <^> because of the order of the arguments.


Here's some usage examples taken from the refactoring in this PR:

// curried map()
anyOf(input) |> map(prepend(match))
map(prepend(match)) <| anyOf(input)
map(prepend(match))(anyOf(input))
anyOf(input) |> map { [match] + $0 }

// <^>
prepend(match) <^> anyOf(input)

// -->
anyOf(input) --> prepend(match)

Ultimately I think that curried map() probably works best in conjuction with |> and trailing closures. In this case |> map {} is basically identical to --> and the difference probably amounts to a stylistic choice (or whether you're importing Prelude).

I'm partial to the Haskell style of <^>. I like how the example reads: "prepend match over any of input" (more or less).

--> is a different style, but I can see an argument for it being more intuitive. Also, we already have it!


At the moment I'm kind of leaning towards sticking with --> and map(), and removing <^> (even though I personally prefer it). Mainly because it ain't broke.

Thoughts?

@robrix
Copy link
Owner

robrix commented May 1, 2015

Upon reflection, I like the spaceship operator, and I’d like to keep it in. What do you think about @neilpa’s suggestion re: resolving #80?

Adam Sharp added 5 commits May 2, 2015 11:52
Used Haskell's `<$>` and `>>=` as an example when choosing the
precedence for `<^>` relative to `>>-`. The value is more or less
arbitrary, but has higher precedence than `>>-`.
@sharplet
Copy link
Collaborator Author

sharplet commented May 2, 2015

@robrix Works for me.

I've pushed 254134a making that change, for your review (and @neilpa's). At this point I've chosen to replace --> with |> map wherever trailing closures are used, and with <^> where named functions are used.

@robrix
Copy link
Owner

robrix commented May 2, 2015

Looks great to me; what do you think, @neilpa?

@neilpa
Copy link
Contributor

neilpa commented May 2, 2015

Awesome 🎉

Not sure if you want to update the mapping examples in the README here or separately.

@sharplet
Copy link
Collaborator Author

sharplet commented May 2, 2015

I'm happy to take a stab at the README here.

@robrix
Copy link
Owner

robrix commented May 2, 2015

🙇 thank you @sharplet!

associativity left

// Higher precedence than `>>-`.
precedence 155
Copy link
Owner

Choose a reason for hiding this comment

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

One potential complication is that Runes doesn’t give a precedence for this operator, meaning it defaults to 100.

I think if a file imports both Runes and Madness they can select which precedence they desire by explicitly redeclaring the operator, so I think that’s ok.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I did check Runes, and felt a little conflicted here. I should create an issue over there, because I think it would be helpful to standardise where possible.

Copy link
Owner

Choose a reason for hiding this comment

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

👌

I don’t think we need to hold up this PR on thoughtbot/Runes#30 but it’s good to have the conversation started.

@robrix
Copy link
Owner

robrix commented May 2, 2015

Just noticed the branch name. I love it!

@sharplet
Copy link
Collaborator Author

sharplet commented May 3, 2015

So after doing some further investigation I've actually just updated thoughtbot/Runes#30 to more strictly follow Haskell's operator precedence. What do you think about doing the same here, i.e.:

/// Flat map operator.
infix operator >>- {
    associativity left
    precedence 100
}

/// Map operator.
infix operator <^> {
    associativity left
    precedence 130
}

@robrix
Copy link
Owner

robrix commented May 3, 2015

If Runes goes for it, we’ll follow suit here 👍

@sharplet
Copy link
Collaborator Author

sharplet commented May 4, 2015

I've just pushed some changes to the playground documentation, so they should all be working fine now. And the operator precedence stuff should also be finalised.

Ok, so one list thing to go:

  • README

😌

@robrix
Copy link
Owner

robrix commented May 4, 2015

Damn, you fixed the playgrounds! Thank you—I had given up on them til the 6.3 betas stabilized and then just forgot.

@robrix
Copy link
Owner

robrix commented May 8, 2015

Any objections to me merging this and picking up the readme later?

@sharplet
Copy link
Collaborator Author

sharplet commented May 8, 2015

robrix added a commit that referenced this pull request May 8, 2015
Add <^> and curried map()
@robrix robrix merged commit 85303c2 into master May 8, 2015
@robrix robrix deleted the mapness branch May 8, 2015 21:35
@robrix
Copy link
Owner

robrix commented May 8, 2015

🎉

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.

<^> ("map") over Parsers
3 participants