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

Replace andThen with flatMap. #910

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@Josef-Vonasek

Josef-Vonasek commented Sep 26, 2017

I find the name flatMap very natural fit for the signature (a -> (Box a)) -> Box a -> Box b. After all the first thing that comes onto my mind, when I see flatMap is flatten and map.

flatten : Box (Box a) -> Box a
map : (a -> b) -> Box a -> Box b

It is then not hard to figure out that it should probably behave like

flatMap function box == flatten (map function box)

andThen on the other hand feels very opaque and / or vague (e.g. |> is very often called andThen) and without documentation I would have no idea would does it do.

@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot Sep 26, 2017

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot commented Sep 26, 2017

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@Josef-Vonasek

This comment has been minimized.

Show comment
Hide comment
@Josef-Vonasek

Josef-Vonasek Sep 26, 2017

The tests are currently failing, but my time is spare.

Is this pull request acceptable? If yes, I will fix the code, so that all test will pass. If not, I am just going to close this pull request.

Josef-Vonasek commented Sep 26, 2017

The tests are currently failing, but my time is spare.

Is this pull request acceptable? If yes, I will fix the code, so that all test will pass. If not, I am just going to close this pull request.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Sep 26, 2017

Member

I considered this name alongside other options when I chose andThen. I do not think it makes sense to make this change.

Member

evancz commented Sep 26, 2017

I considered this name alongside other options when I chose andThen. I do not think it makes sense to make this change.

@evancz evancz closed this Sep 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment