Navigation Menu

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

Document base case of List.for_all and List.exists #9325

Merged
merged 5 commits into from Feb 26, 2020

Conversation

glennsl
Copy link
Contributor

@glennsl glennsl commented Feb 22, 2020

The base case of List.for_all is not immediately obvious to me, so I think it'd be nice to have it documented. And while the base case of List.exists is much more obvious just from its name, I documented the base case of that as well, just for consistency.

@Octachron
Copy link
Member

I think that adding the documentation for the behavior with respect to [] is a good idea.
What do you think of a more declarative wording, something like

For any [p], `List.for_all p []` is always [true]

?

( Note that there are few generic way to remember the base case here. For instance, for a well-behaved f, both for_all f and exists f are monoid morphisms: for_all f from (list,@) to (bool, &&) (i.e. List.forall f (x @ y) = List.forall f x && List.for_all f y ) and exists f from (list,@) to (bool, ||). And (by definition) monoid morphisms map the neutral element from the source monoid to the neutral element of the target monoid. (I am of course not suggesting to add that point to the document))

@glennsl
Copy link
Contributor Author

glennsl commented Feb 23, 2020

Thanks for the explanation! I did actually look up the mathematical definition of universal quantification in predicate logic as well, which said it was a vacuous truth, but that just seems like an axiom. Your explanation actually makes sense.

What do you think of a more declarative wording...

Absolutely. I chose the current wording to try to be consistent with other similar docstrings, for example that of nth_opt:

Return the n-th element of the given list. The first element (head of the list) is at position 0. Return None if the list is too short. Raise Invalid_argument "List.nth" if n is negative.

But just say the word, then I'll change it.

@Octachron
Copy link
Member

You are right, your proposition is more in line in the current style, but maybe merging the two parts would work better:

That is, it returns
   [(p a1) && (p a2) && ... && (p an)] if the list is non-empty
   and [true] otherwise.

or we could swap the two:

That is, it returns [true] is the list is empty and
   [(p a1) && (p a2) && ... && (p an)] otherwise.

@glennsl
Copy link
Contributor Author

glennsl commented Feb 25, 2020

Done, but I made it slightly more explicit:

That is, it returns [(p a1) && (p a2) && ... && (p an)]] for a non-empty list and [true] if the list is empty.

Copy link
Member

@Octachron Octachron left a comment

Choose a reason for hiding this comment

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

It looks good to me. You may want to add a Change entry with your contribution.

stdlib/list.mli Outdated Show resolved Hide resolved
@glennsl
Copy link
Contributor Author

glennsl commented Feb 25, 2020

You may want to add a Change entry with your contribution.

Done. I figured this change was too trivial for that, but I think I got the formalities right now.

@Octachron Octachron merged commit 4823934 into ocaml:trunk Feb 26, 2020
@Octachron
Copy link
Member

Merged, thanks for the contribution!

@glennsl glennsl deleted the patch-1 branch February 26, 2020 09:35
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

3 participants