Two suggestions about Ordering

[user471]

1)Right now there are

sortOn(getKey: ((v:T)=>number)|((v:T)=>string))
minOn(getNumber: (v:T)=>number)

Maybe It should be something like

type NativeOrderable = number | string | boolean
minOn(getKey: (v:T)=> NativeOrderable)
sortOn(getKey: (v:T) => NativeOrderable)

2. There is a common scenario when you want to sort something by two properties

type Foo = {a: number, b: number, c: number}

It's possible to use sortBy

fooList.sortBy((x,y) => arrayCompare([x.a, x.b], [y.a, y.b]))

But I think something like this could be better

sortWith<U>(map: T => U, compare: (v1:U,v2:U)=>Ordering)
fooList.sortWith(x => [x.a, x.b], arrayCompare)

or maybe even

sortOn(getKey: (v:T) => NativeOrderable | U, compare?: (v1:U,v2:U)=>Ordering)

[emmanueltouzery]

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.

[emmanueltouzery]

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.

[emmanueltouzery]

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.

[emmanueltouzery]

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

[user471]

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

[emmanueltouzery]

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 ;-) ).

[user471]

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)

[emmanueltouzery]

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.

[user471]

I think sortOn should also allow desc sort. Maybe with

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

[emmanueltouzery]

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.

[emmanueltouzery]

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

[user471]

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?

[emmanueltouzery]

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.

[emmanueltouzery]

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})

[user471]

thanks

[emmanueltouzery]

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