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.

Sign up for GitHub

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

Jump to bottom

XorT.ensure method #927

Merged
merged 1 commit into from
Mar 16, 2016
Merged

XorT.ensure method #927

merged 1 commit into from
Mar 16, 2016

Conversation

sergeykolbasov
Copy link
Contributor

@sergeykolbasov sergeykolbasov changed the title "ensure" method for XorT XorT.ensure method Mar 10, 2016
@codecov-io
Copy link

Current coverage is 88.94%

Merging #927 into master will not affect coverage as of bd3cae5

@@            master    #927   diff @@
======================================
  Files          179     179       
  Stmts         2125    2126     +1
  Branches        42      42       
  Methods          0       0       
======================================
+ Hit           1890    1891     +1
  Partial          0       0       
  Missed         235     235

Review entire Coverage Diff as of bd3cae5

Powered by Codecov. Updated on successful CI builds.

ceedubs
ceedubs reviewed Mar 11, 2016
View reviewed changes
core/src/main/scala/cats/data/XorT.scala
@@ -51,6 +51,8 @@ final case class XorT[F[_], A, B](value: F[A Xor B]) {

def exists(f: B => Boolean)(implicit F: Functor[F]): F[Boolean] = F.map(value)(_.exists(f))

def ensure[AA >: A](ifLeft: => AA)(f: B => Boolean)(implicit F: Functor[F]): XorT[F, AA, B] = XorT(F.map(value)(_.ensure(ifLeft)(f)))
Copy link
Contributor

Choose a reason for hiding this comment

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

The name ifLeft surprised me. This is the value that is used if the input is a Right but the predicate fails, right? I see that you are just following the name used in Xor, but I'm thinking we should probably change both of these to ifFalse, whenFalse, predicateFailed or something like that. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, in case of predicate falling the whole XorT "falls" to the left branch, it might be an explanation :)

But no problem, I could rename both.

@ceedubs
Copy link
Contributor

ceedubs commented Mar 11, 2016

Thanks, @imliar! I like this. I left one minor comment and am interested in your thoughts on it.

@sergeykolbasov
Copy link
Contributor Author

@ceedubs Done

@ceedubs
Copy link
Contributor

ceedubs commented Mar 13, 2016

@imliar great, thanks! 👍

Are people happy with onFailure for the parameter name?

@stew
Copy link
Contributor

stew commented Mar 16, 2016

👍

stew added a commit that referenced this pull request Mar 16, 2016
@stew
Merge pull request #927 from ImLiar/xort-ensure
8edf0a3 
XorT.ensure method
@stew stew merged commit 8edf0a3 into typelevel:master Mar 16, 2016
@sergeykolbasov sergeykolbasov deleted the xort-ensure branch March 16, 2016 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sergeykolbasov @codecov-io @ceedubs @stew