-
Notifications
You must be signed in to change notification settings - Fork 30
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
Comments
Agreed, both on adding them in more places, and about the naming 👍 |
Great, I can create a PR for that in the next days. |
I was just about to ask for this! |
Please have a look at the above draft PR. What other modules would you like to see |
I have some thoughts about this. Will post today. |
There are a few issues:
My original gripe was that there are lots of times I need a But now that we have
For functions to construct and make use of these
I don't think we need This
We could make it easier to build
I'm not sure what we should do. Hopefully this provides some higher-level thinking on it. |
I know when If we do NOT create a new One more idea to provide a bit more flexibility without introducing a
|
I would keep things lean and not add a
If I understand
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 In my implementation, I chose the latter because
|
Yes I’m glad you understand what I’m saying. Before something goes in the library we should have some sample code showing it being used. I totally get data first for nearly everything. I use it all the time for pipelining. I wonder if I’m missing something about cmp/compare because it is hard to figure out how to use, even though it is in Belt that way. Maybe someone with more experience would know. I could put a question on the forum about this.
I think the issue is we really need cmp/compare functions as an end result - just like we need arrays and options and ints - much more than we need to apply it once in a single pipeline instance with data first. If you really want to do this kind of thing in pipeline mode - fully applied - I think we’re better off with functions like lessThan that returns a bool. And max min greaterThan equals etc.
To this end I think it is a good idea to add a new Compare module. In a Compare module there wouldn’t be a focus on partial application. The code is pretty trivial. Fp-ts has this and it is the most popular functional programming library for JavaScript.
…________________________________
From: Christoph Knittel ***@***.***>
Sent: Wednesday, March 8, 2023 2:24 AM
To: rescript-association/rescript-core ***@***.***>
Cc: jmagaram ***@***.***>; Comment ***@***.***>
Subject: Re: [rescript-association/rescript-core] compare and equal (Issue #77)
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)<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Frescript-association%2Frescript-core%2Fpull%2F84%23issuecomment-1457668297&data=05%7C01%7C%7C7d0e288270494c60417e08db1fbf4695%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638138678607342422%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=NgSZFhJUWFBCisTBToA2qrq9Ftqp%2BncFDQWgNoSLTfs%3D&reserved=0> 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.
—
Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Frescript-association%2Frescript-core%2Fissues%2F77%23issuecomment-1459945200&data=05%7C01%7C%7C7d0e288270494c60417e08db1fbf4695%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638138678607342422%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6XmY7dYgM8nOB0ASV8JD3WVDD%2BnXi6u1oN2Y6NEogqY%3D&reserved=0>, or unsubscribe<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADLZYSYWCPFNNWQCDKZKAL3W3BM5DANCNFSM6AAAAAAVKLNKPA&data=05%7C01%7C%7C7d0e288270494c60417e08db1fbf4695%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638138678607342422%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=4%2B8EF6FvnahKD2u8Gdh63LXHJLkJsviK8iH53eIoUCA%3D&reserved=0>.
You are receiving this because you commented.Message ID: ***@***.***>
|
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 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
} |
Ok I understand better. To be really consistent aren't these the options?
The places where you need a We could make this more discoverable and easier with a
And this would be used like this, but would require that weird
It is so much more flexible to have that
And used like...
I've said enough about all this. I'm ok with adding |
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. |
Currently, we have
cmp
andeq
for theList
,Option
andResult
modules (coming from Belt).It would be good to have these available for more modules, e.g.
Array
,Int
,Float
,String
,Date
,Null
orNullable
.Naming-wise, maybe
compare
andequal
would be more "ReScript-like".The text was updated successfully, but these errors were encountered: