About Ordering module

[jmagaram]

I see there is a new Ordering module that defines the return type for ordering functions. I'm wondering if we would get more flexibility by defining Ordering.t as the comparison function (not just the return value) similar to the code below and as seen in https://github.com/gcanti/fp-ts/blob/master/src/Ord.ts

type t<'a> = ('a, 'a) => int // (or float)

let eval = (t, x, y) => t(x, y)
let map = (f, t, x, y) => eval(t, f(x), f(y))
let fromFloat = (cmp, x, y) => cmp(x, y)->Belt.Int.fromFloat
let reverse = (t, x, y) => eval(t, y, x)

let eq = (t, x, y) => eval(t, x, y) === 0
let neq = (t, x, y) => eval(t, x, y) !== 0

let lt = (t, x, y) => eval(t, x, y) < 0
let lte = (t, x, y) => eval(t, x, y) <= 0
let gt = (t, x, y) => eval(t, x, y) > 0
let gte = (t, x, y) => eval(t, x, y) >= 0

let min = (t, x, y) => lte(t, x, y) ? x : y
let max = (t, x, y) => lte(t, x, y) ? y : x

let clamp = (t, low, high) => v => min(t, high, max(t, low, v))

The primary consumers of these comparison functions are (1) Array.sort, and (2) constructing a Belt Set and Map. Here's how you could use it. I think this approach makes it easier to sort/compare objects by specific properties because it is easier to construct a comparison function via map. Once you have a Ordering.t function, it becomes trivial to modify it with reverse or use it with pairs of items with min, max, gte, neq. The Belt data structures need an int returning function and we can provide a converter in this module so functions that return a float can be used there as well.

module Person = {
  type person = {
    name: string,
    age: int,
  }

  let getAge = i => i.age
}

// Ordering.t is a function
let youngestFirst = Ordering.map(Person.getAge, Int32.compare)
let oldestFirst = Ordering.map(Person.getAge, Int32.compare)->Ordering.reverse
let sortPeopleOldestFirst = people => people->Js.Array2.sortInPlaceWith(oldestFirst)
let oldestOfTwo = (a, b) => Ordering.max(oldestFirst, a, b)

// Other way
let youngestFirst2 = (a, b) => Int32.compare(a->Person.getAge, b->Person.getAge)
let sortPeopleYoungestFirst = people => people->Js.Array2.sortInPlaceWith(youngestFirst2)
let youngestOfTwo = (a, b) => Int32.compare(a->Person.getAge, b->Person.getAge) <= 0 ? a : b

Summary of benefits of this approach

1. We provide various built-in comparison functions like String.compare, Date.compare, and Float.compare. But the moment you want to sort in reverse, you need to write your own function. It is simpler to do something like String.compare->Ordering.reverse to tweak/modify the existing function. This gives you a function you can easily plug into Array.sort and other places.
2. There are libraries that require a comparison function that returns an int, like when building a Belt Set or Map. It is easier to tweak/modify the existing function to make it work in these contexts; just call Date.compare->Ordering.toInt
3. It is extremely common to want to sort an object based on a projection/key like in my example above for sorting people by age. It is convenient to call a function like Ordering.map to construct a new comparison function. This is supported by the TypeScript fp-ts library and is similar to the Rust maxByKey function - https://doc.rust-lang.org/std/cmp/trait.Ord.html.
4. Once you have a comparison function, there are many useful things to do with it that are not supported by the current Ordering module. Things like min, max, clamp, gt, gte, eq, etc.

[jmagaram]

See https://doc.rust-lang.org/std/cmp/index.html for the ordering module in Rust

See docs for the TypeScript version in fp-ts
https://gcanti.github.io/fp-ts/modules/Ord.ts.html

[jmagaram]

@zth what do you think?

[zth]

Will add my thoughts soon. Meanwhile, pinging in @cknitt and @glennsl who I think both has a better idea of the Ordering module than me.

[glennsl]

The proposal here doesn't seem meaningfully different from the last time this was discussed (#77), and my opinion hasn't really changed either. To briefly reiterate, I think the small bit of convenience it adds is dwarfed by the additional complexity and high barrier to learning,. It's better suited for an opinionated third-party library.

[jmagaram]

Nothing is stopping programmers from avoiding the “complexity” of my proposal and writing their own comparison functions. I’m just saying that if we’re going to have an Ordering module we might as well make it a bit more useful. Otherwise we’re guaranteed that people who want to use Belt Set or Map, or want to sort in reverse, or sort an object by key, etc. MUST write their own comparison function. What is so complex about my proposal here? Where is the "high barrier to learning"? I'm an ok programmer and am pretty sure I could easily grok all this, especially if there was a single example in the API help. Not only would I grok it, I'd be thankful to the ReScript and Core team for making it so much easier to do various forms of sorting, max, min, etc.

I think your Ordering module might make things more confusing for Javascript programmers. Before if they wanted to sort an array they knew they needed to provide a function that returns a number, with the sign indicating relative order. Now they see a function that returns an Ordering.t and have to inspect what that is. You wrote somewhere that the idiomatic compare function is subtraction and this will certainly NOT yield a value like Ordering.less. So now I wonder whether I can use subtraction at all. There is no API documentation help. Is eliminating magic numbers like -1.0 enough of a benefit to justify a whole new module?

You have a strong opinion about what should go in Core. I think we should ALSO give serious consideration to the smart teams of people who built Rust, fp-ts and others. My proposal was never seriously “discussed”. There were a handful of comments in a PR from a very small number of people. This is what a thoughtful discussion looks like for the C# language: https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-01-12.md I have raised important issues and opportunities that deserve more than a quick "too opinionated" dismissal. You have declined to address ANY of the specific concerns and ignored ALL the proposed benefits. Your primary objection seemed to be that it wouldn't get used that much and would be an extra thing for people to "wade through". But now that you've created such a module I think it is fair and productive to question the scope of that module.

I assume some of the push back is something like "Rescript is just a better Typescript". But it is also, and based on its ancestry, a functional programming language. From this perspective, an Ordering module that converts functions into other functions fits perfectly well, which is why you see this kind of thing in Haskell and fp-ts. Another argument is that Core is just a 1-1 wrapper around JavaScript. If this is true we should remove Result, remove Option and use Nullable instead, make Array.push return an int, etc. I prefer to embrace the fact that ReScript and Core provides a practical balance between lightweight functional programming concepts and JavaScript and figure out how to elegantly combine them.

[zth]

I agree with @glennsl. I do appreciate the effort to simplify, but I'm not confident it'll achieve its goal. Therefore I also think this fits better in an opinionated 3rd party lib for starters. We can revisit when that lib has been used for a while and there's a clear view of what the users of that lib thinks of it, whether it improves the situation or not.

I feel like we've discussed the rest at length in other PRs and issues. This includes us not having the same view of what to consider for Core, how functional it should be, and so on.

I think continuing adding these types of things to rescript-extras is a good approach. That way you can shape your "Core extension" just as you'd like, and you're free to incorporate any opinionated FP direction you'd like, using fp-ts and all of the other projects you've mentioned.