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

compare and equal #77

Open
cknitt opened this issue Feb 28, 2023 · 12 comments
Open

compare and equal #77

cknitt opened this issue Feb 28, 2023 · 12 comments

Comments

@cknitt
Copy link
Collaborator

cknitt commented Feb 28, 2023

Currently, we have cmp and eq for the List, Option and Result modules (coming from Belt).

It would be good to have these available for more modules, e.g. Array, Int, Float, String, Date, Null or Nullable.

Naming-wise, maybe compare and equal would be more "ReScript-like".

@zth
Copy link
Collaborator

zth commented Feb 28, 2023

Agreed, both on adding them in more places, and about the naming 👍

@cknitt
Copy link
Collaborator Author

cknitt commented Feb 28, 2023

Great, I can create a PR for that in the next days.

@jmagaram
Copy link
Contributor

jmagaram commented Mar 2, 2023

I was just about to ask for this!

@cknitt cknitt mentioned this issue Mar 3, 2023
9 tasks
@cknitt
Copy link
Collaborator Author

cknitt commented Mar 3, 2023

Please have a look at the above draft PR.

What other modules would you like to see compare and equal for?

@jmagaram
Copy link
Contributor

jmagaram commented Mar 4, 2023

I have some thoughts about this. Will post today.

@jmagaram
Copy link
Contributor

jmagaram commented Mar 4, 2023

There are a few issues:

  • cmp functions, those like ('a,'a)=>int. Should we have built-in versions for int, float, etc.?
  • utilities that construct/map cmp functions
  • utilities that make practical use of cmp, like equals, lessThan, max

My original gripe was that there are lots of times I need a cmp - sorting an array or making a Belt.Map - and I find myself writing an inline ternary like [3,2,7]->Array.sort((x,y)=>x<y ? -1 : x>y ? 1 : 0). Annoying. Is this what everyone does? It would be convenient if I could do [3,2,7]->Array.sort(Int.cmp). So maybe add these cmp functions to the types it makes sense like int, float, date, bigInt. bool wouldn't hurt and good for consistency. string is tricky because there are many ways to compare. I didn't realize Javascript has a way of comparing strings case insensitive, ignoring accents, numeric like "1" < "10". but we could have a default String.cmp which is like string.localeCompare (which returns a float by the way) but with no options - a quick and dirty way to sort some strings in a reasonable way.

But now that we have cmp for built-in primitives, what if you want to do a reverse sort? We could have Int.cmp and Int.cmpReverse? I think if we're going to have one we should have both.

Belt.Option.cmp seems a little weird to me. If you want to build a cmp to compare two options I think you'd want a function that takes a ('a, 'a) => int as input, and generates a (option<'a>, option<'a>) => int as output. And then feed that result into Array.sort or something. I don't know how you'd use the function as it is designed now.

For functions to construct and make use of these cmp, see ord in fp-ts for ideas. We could introduce a Cmp module.

module Cmp = {
  type t<'a> = ('a, 'a) => int
  let eval = (t: t<'a>, x, y) => t(x, y)
  let reverse = (t, x, y) => eval(t, y, x)
  let lessThan = (t, x, y) => eval(t, x, y) < 0
  let lessThanOrEqual = (t, x, y) => eval(t, x, y) <= 0
  let greaterThan = (t, x, y) => eval(t, x, y) > 0
  let greaterThanOrEqual = (t, x, y) => eval(t, x, y) >= 0
  let equal = (t, x, y) => eval(t, x, y) === 0
  let notEqual = (t, x, y) => eval(t, x, y) !== 0
  let max = (t, x, y) => lessThan(t, x, y) ? y : x
  let min = (t, x, y) => lessThan(t, x, y) ? x : y
  let using = (f, t, x, y) => eval(t, f(x), f(y))
  let between = (t, min, max, i) => lessThanOrEqual(t, min, i) && lessThanOrEqual(t, i, max)
  let clamp = (t, min, max, i) => lessThan(t, i, min) ? min : greaterThan(t, i, max) ? max : i
}

Cmp.using is particularly useful to build a cmp you can use to compare bigger types based on their contents. So you can compare two options based on their contents. Or if you had two person objects, you could compare based on the people's age. And use reverse if you want to do a reverse sort. For example...

module Person = {
  type t = {name: string, age: int}
  let getAge = i => i.age
}

let oldest = (p1, p2) => Person.getAge->Cmp.using(Int.cmp)->Cmp.max(p1, p2)
let oldestFirst = (people: array<Person.t>) =>
  people->Array.sortInPlace(Person.getAge->Cmp.using(Int.cmp)->Cmp.reverse)
let sortOptions = (nums: array<option<int>>) => nums->Array.sortInPlace(Option.cmp(Int.cmp))

I don't think we need equals on arrays. It might help in pipeline mode to do array1->Array.equals(array2). There are tons of useful functions where you'd take two arrays as input and want something out. Definitely don't think we need some kind of compare on arrays. Same for lists.

This Cmp module idea, like Ord from fp-ts and a similar things in Haskell, might be too nerdy/functional, but it is definitely useful. Without it you can compare people by doing something like this below, but then you still don't have utilities for max, min, lessThan, reverse, etc.

let youngestFirstCmp = (p1, p2) => Int.cmp(p1->Person.getAge, p2->Person.getAge)

We could make it easier to build cmp with maps.

module type Int = {
  let cmp: (int, int) => int
  let cmpReverse: (int, int) => int
  let cmpUsing: ('a => int, 'a, 'a) => int
  let cmpUsingReverse: ('a => int, 'a, 'a) => int
}

let younger = Int.cmpUsing(Person.getAge)

I'm not sure what we should do. Hopefully this provides some higher-level thinking on it.

@jmagaram
Copy link
Contributor

jmagaram commented Mar 4, 2023

I know when cmp is used - like in array sorting and making a set/map. When is eq used?

If we do NOT create a new Cmp module, I and other people who want flexibility with cmp can make our own and include Cmp.int and Cmp.intReversed etc. inside as well as all the Cmp.using, Cmp.reverse, Cmp.max. So I don't think anything here is really required. At a minimum it would be nice if the built-in primitive modules had Int.cmp, Float.cmp, Date.cmp, BigInt.cmp, String.cmp, Bool.cmp, which worked from smallest to largest, so no one has to write that ternary and programmers can do a natural sort without any hassle. For Option, Array, List, Undefined I'd do it where the output of the function is the cmp; that way you can plug it easily into wherever it is needed.

One more idea to provide a bit more flexibility without introducing a Cmp module and making things a bit discoverable for the programmer. We could have different options for different types.

module Int = {
  type cmpBy<'a> =
    | MinToMax
    | MaxToMin
    | MinToMaxWith('a => int)
    | MaxToMinWith('a => int)
  let cmp = (a, b) => a < b ? -1 : a > b ? 1 : 0
  let cmpBy = options => {
    switch options {
    | MinToMax => cmp
    | MaxToMin => (a, b) => cmp(b, a)
    | MinToMaxWith(f) => (a, b) => cmp(f(a), f(b))
    | MaxToMinWith(f) => (a, b) => cmp(f(b), f(a))
    }
  }
}

let youngestFirst = Int.cmpBy(MinToMaxWith(Person.getAge))

@cknitt
Copy link
Collaborator Author

cknitt commented Mar 8, 2023

I would keep things lean and not add a Cmp module. As you said

I and other people who want flexibility with cmp can make our own and include Cmp.int and Cmp.intReversed etc. inside as well as all the Cmp.using, Cmp.reverse, Cmp.max. So I don't think anything here is really required.

If I understand

For Option, Array, List, Undefined I'd do it where the output of the function is the cmp; that way you can plug it easily into wherever it is needed.

and your comment in #84 (comment) correctly, you are saying that you would like to have

Array.compare(cmp, a, b)

instead of

Array.compare(a, b, cmp)

so that you can do things like x->Array.sort(Array.compare(Int.compare))?

In my implementation, I chose the latter because

  1. it matches the way it is done in Belt
  2. you can use it with pipe first like arrayA->Array.compare(arrayB, cmp) similar to other "methods" in the Array module
  3. I don't know if we should optimize for curried application with uncurried always/uncurried by default coming with ReScript 11.

@jmagaram
Copy link
Contributor

jmagaram commented Mar 8, 2023 via email

@glennsl
Copy link
Contributor

glennsl commented Mar 8, 2023

To make the proposals we're talking about properly comparable:

Current design:

x->Array.sort((a, b) => a->Array.compare(b, Int.compare))

Currying:

x->Array.sort(Array.compare(Int.compare))

I don't see much use for a Cmp module personally, but I do think an Ord module could be quite useful to make intent more explicit. So that values and operations can be named properly instead of using magic numbers and weird arithmetic tricks in comparison functions.

module Ord = {
  let t = int

  let lessThan = -1
  let equal = 0
  let graterThan = 1

  let isLessThan = ord => ord < 0
  let isEqual = ord => ord == 0
  let isGreaterThan = ord => ord > 0

  let invert = ord => -ord
}

@jmagaram
Copy link
Contributor

jmagaram commented Mar 8, 2023

Ok I understand better. To be really consistent aren't these the options?

[[1, 2, 3], [1, 2, 4]]->Array.sortInPlace((i, j) => i->Array.cmp(j, (x, y) => x->Int.cmp(y)))
[[1, 2, 3], [1, 2, 4]]->Array.sortInPlace(Array.cmp(Int.cmp))

The places where you need a cmp function you can "cheat" and put in Int.cmp and this keeps the code very concise. As soon as you want a reverse sort or to pre-process the data like sorting on the absolute value of a float or comparing strings in a case-insensitive way, you've got to write it out like (a,b)=>mapper(a)->compare_function(mapper(b)).

We could make this more discoverable and easier with a compareMap function.

module type FloatType = {
  let compare: (float, float) => int
  let compareMap: (~map: float => float=?, ~reverse: bool=?, unit, float, float) => int
}

And this would be used like this, but would require that weird unit parameter on the end if we have optional parameters.

  let x1 = nums->Array.sort(Float.compareMap(~map=Math.abs, ~reverse=false, ()))
  let x2 = nums->Array.sort(Float.compareMap(~reverse=true, ()))

It is so much more flexible to have that Compare or Ord or Cmp module. The code is trivial. And it makes it easy to compare/sort larger data structures.

module type Cmp = {
  type t<'a> = ('a, 'a) => int
  let reverse: t<'a> => t<'a>
  let map: (t<'a>, 'a => 'a) => t<'a>
  let fromMap: ('a => 'b, t<'b>) => t<'a>
  let clamp: (t<'a>, ~min:'a, ~max:'a) => 'a
  let min: (t<'a>, 'a, 'a) => 'a
  // others like less than, greater than, max
}

And used like...

  let nums = [1.0, 3.4, 7.2, 8.6, 2.4]
  let x1 = nums->Array.sort(Float.cmp->Cmp.reverse->Cmp.map(Math.ceil))

I've said enough about all this. I'm ok with adding compare and equals as originally planned even for array and list. I can augment this with my own Compare module. I prefer the shorter cmp and eq naming because these are technical things. No one expects the result of a compare to be a -1, 1, or 1. It also follows the functional programming patterns used in Haskell, fp-ts. Rust calls it ord. Belt uses cmp to construct a set or map. When you string together a bunch of these functions in a pipeline the longer Compare gets pretty noisy.

@glennsl
Copy link
Contributor

glennsl commented Mar 8, 2023

To be really consistent aren't these the options?

[[1, 2, 3], [1, 2, 4]]->Array.sortInPlace((i, j) => i->Array.cmp(j, (x, y) => x->Int.cmp(y)))
[[1, 2, 3], [1, 2, 4]]->Array.sortInPlace(Array.cmp(Int.cmp))

x->Int.cmp(y) is the same as Int.cmp(x, y), therefore (x, y) => x->Int.cmp(y) is equivalent to just Int.cmp. It's only when there are additional arguments at the start that you'd have to wrap it in an explicit function.

It is so much more flexible to have that Compare or Ord or Cmp module.

I'm not against such a module, but I don't think it will be used so frequently that most people will bother to learn it instead of just writing out the function. And then it becomes more of a burden in a library like this. Just stuff you have to wade through to get to the important bits that you actually need. It isn't essential, but more of a utility module that can easily be added by a third party library with a focus on more holistic functional abstractions.

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