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 Validated.andThen #611

Merged
merged 2 commits into from
Nov 10, 2015
Merged

Conversation

ceedubs
Copy link
Contributor

@ceedubs ceedubs commented Nov 7, 2015

Fixes #604.

This is essentially flatMap for Validated, but since flatMap
suggests monadic bind in Cats and this is not consistent with ap,
another name is being used.

This PR is not complete yet. I need to add a Scaladoc comment and tut documentation about this change. However, I figured there might be some discussion about the name of the method, so I want to wait until a name is decided to start writing docs :). Is andThen too overloaded?

Fixes typelevel#604.

This is essentially `flatMap` for `Validated`, but since `flatMap`
suggests monadic bind in Cats and this is not consistent with `ap`,
another name is being used.
@codecov-io
Copy link

Current coverage is 76.04%

Merging #611 into master will decrease coverage by -0.25% as of 249d37c

@@            master    #611   diff @@
======================================
  Files          160     159     -1
  Stmts         2194    2066   -128
  Branches        68      69     +1
  Methods          0       0       
======================================
- Hit           1674    1571   -103
  Partial          0       0       
+ Missed         520     495    -25

Review entire Coverage Diff as of 249d37c

Powered by Codecov. Updated on successful CI builds.

@adelbertc
Copy link
Contributor

andThen is fine by me, but it might confuse people seeing andThen from FunctionN. I know Scoobi used mapFlatten, or at least did at some point..

@mpilquist
Copy link
Member

👍 I'm okay with andThen.

@non
Copy link
Contributor

non commented Nov 8, 2015

👍 I would be fine with andThen.

In the context of Validated I could also imagine calling it something about valid e.g. whenValid, mapValid, etc.

@ceedubs
Copy link
Contributor Author

ceedubs commented Nov 9, 2015

It sounds like people are generally okay with andThen, so I'll go with that. We can reconsider in the future if it's tripping people up.

I'll move forward with documentation.

@ceedubs
Copy link
Contributor Author

ceedubs commented Nov 10, 2015

I added some documentation.

@adelbertc I deleted part of your Validated docs about flatMap, because it didn't seem to belong after the andThen docs, but I'd like your input on this change.

I think there's still room for improvement, so please feel free to chime in.

@adelbertc
Copy link
Contributor

LGTM 👍

adelbertc added a commit that referenced this pull request Nov 10, 2015
@adelbertc adelbertc merged commit c38f89b into typelevel:master Nov 10, 2015
@ceedubs ceedubs deleted the validated-andThen branch November 15, 2015 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants