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

BatList.n_cartesian_product #359

Closed
wistery-k opened this issue Feb 23, 2013 · 5 comments
Closed

BatList.n_cartesian_product #359

wistery-k opened this issue Feb 23, 2013 · 5 comments

Comments

@wistery-k
Copy link
Contributor

Is there any reason for
BatList.n_cartesian_product [] = assert false
?
I think [[]] is good (or at least invalid_arg) instead of assert false.

@gasche
Copy link
Member

gasche commented Feb 23, 2013

Thanks for your report. It is clear that assert false is wrong: assertion failures are to be used in cases that cannot happen, and in particular it should never, ever be sent back to the user. This is clearly a bug.

After a bit of mathematical thinking, I think the One Right Answer in this case is [[]]. Would you care to provide a patch to change this behavior? BatList.n_cartesian_product currently has no tests at all. In your patch, could you write some unit tests verifying the behavior in the general case, for a list of singleton lists, for a list of a single list, and for the empty list?

@wistery-k
Copy link
Contributor Author

I sended a pull request. (I'm a github beginner. I'm sorry for anything bad..)

@gasche
Copy link
Member

gasche commented Feb 23, 2013

Your contribution closes this bug. Thanks a lot, and I hope you'll find other bugs :-)

@gasche gasche closed this as completed Feb 23, 2013
@murmour
Copy link
Contributor

murmour commented Aug 6, 2023

True story:

In 2012, young Max Mouratov wrote a bunch of code that used n_cartesian_product for "give me all combinations of items from the given lists, so I can do something useful to each of them".

In 2013, young Gabriel Scherer found the One Right Answer to "what should n_cartesian_product [] be", and wrote a deep blog post about it: http://gallium.inria.fr/blog/on-the-nary-cartesian-product/.

In 2023, the bunch of code that young Max wrote in 2013 suddenly broke on a particular input, and the now old Max discovered that n_cartesian_product [] returns [[]], and not [] as the young Max had expected (by intuition). But [] was not "something useful" in all the code that young Max wrote, and the old Max sympathized with the young one.

Then, the old Max read the deep blog post by the young Gabriel, and his IQ increased by one point. He understood the secrets of the universe, and the whole truth unfolded before him.

The blog post ended with a bit of infinite wisdom:

No theoretical considerations make as strong an argument as this one: the right answer is the one that makes the code elegant.

So, the old Max pondered about it for a while, and his mind got illuminated by the brightest idea ever. He copied the implementation of n_cartesian_product from Batteries and replaced [[]] with [] inside it, because it was an elegant and pragmatic solution that spared him the need to scatter if l = [] throughout his client code, across all of his programs.

There is no morale.

But the old Max from 2023 suggests that perhaps it'd be wise to document this corner case in the docstring of n_cartesian_product, because its behaviour is not quite intuitive, and not quite obvious to those programmers who aren't type theorists. He also wonders if the old and wise Gabriel from 2023 has any new thoughts on the matter now.

@UnixJunkie
Copy link
Member

If you send an improvement for the ocamldoc of this function, I will gladly merge it.

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

No branches or pull requests

4 participants