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

equal functions in Enum and Seq #809

Open
mars0i opened this issue Nov 27, 2017 · 8 comments
Open

equal functions in Enum and Seq #809

mars0i opened this issue Nov 27, 2017 · 8 comments

Comments

@mars0i
Copy link
Contributor

mars0i commented Nov 27, 2017

LazyList lacks an equal predicate. Both Enum and Seq include equal, but the syntax is different: Enum has a required equality-predicate parameter for comparison of individual elements, while Seq instead has an optional argument that defaults to =.

I want to submit a PR for equal for LazyList, but I'm not sure which model to follow. I think the optional argument version of Seq makes more sense, since any of the usual standard OCaml types, except functions, will automatically work with =. In fact I was considering submitting code more or less identical to what's in Seq, following kantian's suggestions in this thread in discuss.ocaml.org.

It would be nice if all three equal functions had the same syntax, but I gather that Enum is used a lot in Batteries, so changing Enum's equal might break a lot of things. I don't know how heavily Seq is used, and whether changing its syntax might break things.

Given the difference between Enum's and Seq's equal functions, I'm not sure how best to proceed.

@gasche
Copy link
Member

gasche commented Nov 27, 2017

If you grep for val equal in the repository, you will see that having a mandatory argument is much more common than having an optional argument, so I would use that for consistency.

@mars0i
Copy link
Contributor Author

mars0i commented Nov 28, 2017

OK. Should I "fix" Seq's equal, too? Or is that risky?

@UnixJunkie
Copy link
Member

@c-cube

@gasche
Copy link
Member

gasche commented Nov 28, 2017

You shouldn't change the interface of an existing function unless we do a major release. You could send this change, separately, to the v3 branch -- which we need to rebase on top of the current master, but that is another story.

@mars0i
Copy link
Contributor Author

mars0i commented Nov 29, 2017

If equal has a required eq argument, it's the same function as for_all2, except that it returns false rather than raising an exception if one list is longer and the test succeeds up to that point.

Seq and Enum don't have functions named for_all2, but Enum's equal can also be used like for_all2 if the enumerations are the same length. You could do the same thing with Seq.equal; the function parameter defaults to =, but you could pass any function with the right signature.

I'm just a little befuddled, confused at the moment about what functions are needed. An equal function with a required predicate argument is essentially for_all2, with false if the lists are different lengths. Are both worth having? I certainly like the idea of an equal function, and would write my own if it wasn't provided in LazyList. But it's a somewhat strange idea that an arbitrary function could be passed to equal, and that it's almost the same function as for_all2.

In the abstract I feel that Seq's default test predicate syntax is better--that makes the name equal more appropriate--but I see the logic of following what's mostly done in the rest of the code.

@gasche
Copy link
Member

gasche commented Nov 29, 2017

Yes, I personally think that it is fine having two functions doing the same thing with different names that carry different connotations. (Furthermore, in this case, they are not exactly doing the same thing. Note that I wouldn't encourage people to use equal whenever they need "for_all2 on unequal lengths" because that would hurt code readability.)

Two points on the optional arguments:

  • polymorphic comparison is arguably a dangerous feature of the OCaml language; it is convenient and used for convenience, but there is a reasonable argument that we should make it easy to not use it, and in particular not bake it implicitly in our APIs
  • modular implicits (type-classes for OCaml), if/when they eventually come to the language, will force us to revisit all this design space in any case

@mars0i
Copy link
Contributor Author

mars0i commented Nov 29, 2017

OK. That sounds fine. Maybe I'll note the similairity in the doc comments.

I.e. you're saying that having = as a default would have an undesirable aspect, too, I gather.

@UnixJunkie UnixJunkie changed the title equal functions in Enum, Seq, and LazyList equal functions in Enum and Seq Jul 31, 2019
@UnixJunkie
Copy link
Member

I updated the issue title, because the LazyList part was done.
I guess others are still welcome.

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

3 participants