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

Disallow (maybe/just nil) #149

Merged
4 commits merged into from
Feb 16, 2016
Merged

Disallow (maybe/just nil) #149

4 commits merged into from
Feb 16, 2016

Conversation

yurrriq
Copy link
Collaborator

@yurrriq yurrriq commented Feb 16, 2016

Update #'cats.core.monad.maybe/just

Remove nullary clause and add a precondition to the remaining unary clause to ensure v is non-nil.
Update the docsctring accordingly.

Update 'cats.monad.maybe

In -mreturn, if v is nil, return #<Nothing>, otherwise #<Just v>.

Update #'cats.core/guard

(return true) instead of (return nil)

Fixes #148.

Remove nullary clause, since that doesn't make much sense anyway, and
add a precondition to the remaining unary clause to ensure v is non-nil.

Update the docsctring accordingly.
In -mreturn, if v is nil, return #<Nothing>, otherwise #<Just v>.
(return b) instead of (return nil)

This is mostly so the maybe tests pass and could be very misguided.
@yurrriq yurrriq changed the title Disallow (just nil) Disallow (maybe/just nil) Feb 16, 2016
@ghost
Copy link

ghost commented Feb 16, 2016

I think we better (return true) in guard's body, since people maybe nil-punning the condition of the predicate.

(return true) per @dialelo's suggestion.

#149 (comment)
@yurrriq
Copy link
Collaborator Author

yurrriq commented Feb 16, 2016

Sounds good to me and the tests passed locally. I'm actually going to bed now... 💤

@ghost
Copy link

ghost commented Feb 16, 2016

Sweet dreams 😴

ghost pushed a commit that referenced this pull request Feb 16, 2016
@ghost ghost merged commit 16b9dfe into master Feb 16, 2016
@ghost ghost deleted the disallow-just-nil branch February 16, 2016 10:57
hojacartaft added a commit to hojacartaft/cats that referenced this pull request Aug 25, 2024
This pull request was closed.
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.

1 participant