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

Update any?, every? #1257

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Update any?, every? #1257

merged 1 commit into from
Aug 18, 2023

Conversation

primo-ppcg
Copy link
Contributor

@primo-ppcg primo-ppcg commented Aug 18, 2023

Updates any? and every? to be exact functional analogues to or and and.

  • any? returns the final value if all preceding values are falsey, rather than nil.
  • every? returns the final value if all preceding values are truthy, rather than true.

I believe this makes their definitions more coherent. any? would sometimes return an element from the collection (if it were truthy), and sometimes nil. every? would sometimes return an element from the collection (if it were falsey), and sometimes true. After, each will always return an element from the collection.

As their meaning will not have changed when used as a predicate, I believe these changes can be made with minimal fallout. every? in particular is much more useful this way. Because of the spatial separation in the documentation, I have duplicated the docstrings from or and and.

For the interested, the implementation is approximately 10% faster for each.

Updates `any?` and `every?` to be exact functional analogues to `or` and `and`.
Copy link
Member

@pepe pepe left a comment

Choose a reason for hiding this comment

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

LGTM

@bakpakin bakpakin merged commit 2ac36a0 into janet-lang:master Aug 18, 2023
8 checks passed
@primo-ppcg primo-ppcg deleted the any-every branch August 18, 2023 12:22
@sogaiu
Copy link
Contributor

sogaiu commented Aug 18, 2023

It wasn't really in real-world usage, but FWIW I did make this change after this PR got merged.

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.

None yet

4 participants