Code Monkey home page Code Monkey logo

Comments (16)

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

Thanks for the feedback! Regarding #1 actually the current solution is more type-safe. With the current solution you must give a lambda which will return always string or always number.
With your type, the lambda could return string for one value, number for another, and boolean for another.

But yes we need to allow returning strings for minOn (and maxOn etc) besides number. We can probably have a type alias like

type SortCallback = ((v:T)=>number)|((v:T)=>string)

Although I'm not sure if it's worth it. I'm not sure allowing booleans buys us much though.

I'll think about #2 and answer soon on that one too.

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

I've now commited to master ToOrderable and it's used for maxOn & minOn as well:
7ea60a7

Let me know what you think about that. I'll look at the second part of your bug ASAP.

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

so, my plan for the second issue is to keep the sorting functions as they are (I'm already not so happy about having both sortOn and sortBy). But prelude would offer a new function:

export function fieldsOrderingFn(
...fieldReaders: Array<((obj:T)=>string)|((obj:T)=>number)>): (v1:T,v2:T)=>Ordering {

Prelude already exports slightly similar functions, like fieldsHashCode:
http://emmanueltouzery.github.io/prelude.ts/latest/apidoc/files/comparison.html#fieldshashcode
https://github.com/emmanueltouzery/prelude.ts/wiki/Equality

I basically coded it now, but I need to write apidoc & tests. In your case that would enable:

fooList.sortBy(fieldsOrderingFn<Foo>(x=>x.a, x=>x.b))

Let me know if you think there's a flaw with that idea. Otherwise I'll push that today I think.

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

I'm also interested if you have another idea for the name of that new function.

from prelude-ts.

user471 avatar user471 commented on August 27, 2024

ToOrderable

I would add booleans. If someone what to sort by predicate why not? Put something to the end or to the start of the list is pretty reasonable case
users.sortOn(x => x.email.endsWith("@gmail.com"))

Does fieldsOrderingFn<Foo>(x=>x.a, x=>x.b) have any advantage over fieldsOrderingFn<Foo>(x=>[x.a, x.b])? I think it would be easy to read, especially if there is more than two fields.

I'm also interested if you have another idea for the name of that new function.

Javascript array sort method uses compareFn name so maybe fieldsCompareFn

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

Ok you convinced me about adding booleans for the sorting, I'll add it. My first thought was, if you're interested in sorting with endsWith then Seq.partition may be what you're looking for. But in the context of multi-field sorting as you're mentioning, I can imagine an use case when you'd like to say... Let's put the messages with the 'urgent' bit first, and then we sort by date.
So yes I'll add the boolean for sorting.

Does fieldsOrderingFn(x=>x.a, x=>x.b) have any advantage over fieldsOrderingFn(x=>[x.a, x.b])?

there is an objective advantage, and one that I had in mind when I came up with that, although it's really not a very strong one. That's related to the types, in a bit the same way as I was saying that (x:T)=>string|(x:T)=>number is more precise than (x:T)=>string|number.

With the type that I suggested, you must give a series of lambdas. Each one must return a string. Or a number. While if you give an array, then each element of the array will contain string|number. And there we are again. In theory (and yes it's a bit stretching, but it's possible), the user could provide a function which will return a string for the second element once, and another time a number. And it'll type-check. When I compare element-by-element in prelude's code, I'll have to either cast to any (trust the user was disciplined), or have an if, potentially even decide to throw if I refuse type mismatches.
With the list of lambdas, it's type-safe.
Actually I'd prefer the array too, more because I suspect that at runtime it'd be faster (although these kind of things are fast either way). But I couldn't think of a way to make it as type-safe and so I preferred the safer way. But let me know what you think regarding that. I'm not really concerned about compacity though. I would think sorting by more than 3 fields would be rare, and the spec for each field could be on its line, and so on.

I like fieldsCompareFn for the name! I'm not sure about the Fn postfix though, I may drop it (I know, I'm the one who first added it, but hey ;-) ).

from prelude-ts.

user471 avatar user471 commented on August 27, 2024

My first thought was, if you're interested in sorting with endsWith then Seq.partition may be what you're looking for

Not necessary. For example I just want to send emails to gmail last for some reason.

In theory (and yes it's a bit stretching, but it's possible), the user could provide a function which will return a string for the second element once, and another time a number.

I don't think it would be a problem in real life code and I think the array version looks more idiomatic because of sorting something by tuple, but on the other hand everyone can create their own function like that so library could offer more type-safe version by default.

One thing bother me. fooList.sortBy(fieldsOrderingFn<Foo>(x=>x.a, x=>x.b)) looks very similar to sortOn. Couldn't sortOn just accept more keys?
fooList.sortOn(x => x.a, x => x.b)

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

Actually I started thinking, if we do multi-criteria sorting, inevitably we'll want to sort on some things descending. I'm pretty sure it's possible to achieve:

const sortingFn = compareBy(x=>x.a).thenByDesc(x=>x.b);

with also compareByDesc and thenBy. I'll have a go at it tonight my time, in like 12h, probably.

from prelude-ts.

user471 avatar user471 commented on August 27, 2024

I think sortOn should also allow desc sort. Maybe with

type SortCallbackWithDesc = SortCallback | {desc: SortCallback}
fooList.sortOn({desc: x => x.a})

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

I'd really prefer to have sortOn & sortBy as simple as possible & put that complexity elsewhere. I like the path I'm taking with that. Remember what also you said: with this, you can write your own functions with arrays if you prefer, or whatever, you're not tied to prelude implementing something or not. Also, this complexity must then be replicated in maxOn, minOn, sortOn, and other places presumably.

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

@user471 you're right about sortOn :-) I'll implement it as you suggested 👍

from prelude-ts.

user471 avatar user471 commented on August 27, 2024

I'd really prefer to have sortOn & sortBy as simple as possible & put that complexity elsewhere

I understand that. Complex functions could be overwhelming, but personally I prefer when I press dot type sort and I can see everything that is available and it's still possible to use simple version of the function without any disadvantage.

and other places presumably.

Do you mean other collections? Maybe It is possible to use some mixins so there wouldn't be any duplication?

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

OK I think master now has all the fixes. Let me know if you see something's off. I want to add one more change and then I'll release 0.7.2.

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

About your last comment: you were right about sortOn. Looking at the purpose of sortOn and sortBy it was obvious this did fit in sortOn. I implemented the way you suggested:

list.sortOn(x=>x.a,{desc:x=>x.b})

from prelude-ts.

user471 avatar user471 commented on August 27, 2024

thanks

from prelude-ts.

emmanueltouzery avatar emmanueltouzery commented on August 27, 2024

just published version 0.7.2 with the changes we discussed! thank you for the feedback & suggestions!

from prelude-ts.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.