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

Sort order with multiple keys #414

Closed
luksch opened this issue Dec 1, 2013 · 27 comments
Closed

Sort order with multiple keys #414

luksch opened this issue Dec 1, 2013 · 27 comments

Comments

@luksch
Copy link

luksch commented Dec 1, 2013

Hi, I am a in awe of the power of lodash. For my project it has been of invaluable help so far. However, now I am stuck with a problem with which lodash seems to fall a bit short.

Here is the problem:

I have an array of objects with the same properties, say

[{name:"barney", age: 38},
 {name:"fred", age: 40},
 {name:"fred", age: 12}]

I want to sort the array by name in descending order, but age should be in ascending order.

lodash supports multiple sorting keys by the "pluck" syntax, but only can deal with ascending sorting this way. Is there a way that the sortBy function is altered that it also accepts an array of objects that define the property name (key) and the order direction? I would love to do:

mySortedArr = _.sortby(
    unsortedArr, [
        {propertyName:"name", order:"desc"}, 
        {propertyName:"age", order:"asc"}
    ]);

Any chance something like that enters the library?

@jdalton
Copy link
Member

jdalton commented Dec 1, 2013

Thanks for the encouragement! I'll keep a tab on this to see if it's requested more, but at the moment I don't have plans to expand this to _.sortBy. You can always iterate over the collection from right to left with _.eachRight, _.reduceRight or the other right associative methods.

@jdalton jdalton closed this as completed Dec 1, 2013
@joshbowdoin
Copy link

+1 - would definitely like this!

Syntax could be similar to angular's orderBy filter: https://docs.angularjs.org/api/ng/filter/orderBy, where you'd simply put a '+' (optional) or '-' in front of the property name to control ascending/descending.

@octref
Copy link
Contributor

octref commented Jul 6, 2014

@jdalton
Right associative methods doesn't solve the problem.
For example, suppose this is the result of _.sortBy(myArray, ['foo', 'bar'])

[{'foo': 'A', 'bar': 1}, #1
 {'foo': 'A', 'bar': 2}, #2
 {'foo': 'B', 'bar': 1}, #3
 {'foo': 'B', 'bar': 2}] #4

If I do a reverse, or use right associative methods, I can only get 4-3-2-1.
But what if I want to do a sort with 'foo' ascending and 'bar' descending? The result I want is 2-1-4-3 but I can't seem to achieve that in lodash.

As @joshbowdoin suggests, an optional '-' which indicates descending would be a really nice addition.
Like this _.sortBy(myArray, ['foo', '-bar']) for my example above.

BTW, really nice work. It's saved me tons of hours.

@darthsteedious
Copy link

+1 - would love a _.sortBy(myArray, ['foo', '-bar']) syntax.

@ghost1face
Copy link

+1

@jdalton
Copy link
Member

jdalton commented Jan 7, 2015

I'd dig a proposed implementation to go along with the +1's.
In 3.0 we're separating _.sortBy out into _.sortByAll to handle multiple properties. This lets use keep _.sortBy simple.

@bnjmnt4n bnjmnt4n changed the title sort oder with multiple keys Sort order with multiple keys Jan 9, 2015
@leefernandes
Copy link

+1 for using an order prefix for each sort

@TrejGun
Copy link

TrejGun commented Feb 21, 2015

You always can add _.sortBySome

@rummelsworth
Copy link

I'd love to see sortByAll handle optionally-specified per-criterion descending sort order. (Btw, gotta mention it... Like many others, I am in love with the performance and expressiveness of lodash. Thanks for your awesome work.)

Perhaps a signature something like sortByAll(collection, props, order) for usage like _.sortByAll(myArray, ['foo', 'bar'], [false, true]), to borrow from @octref 's example?

Copying & adapting compareMultipleAscending to compareMultiple seems straightforward (to me at least, relatively new to JS and possibly unaware of some pitfalls):

/**
 * @param {Array} order Each truthy element indicates descending sort order for the corresponding criterion.
 */
function compareMultiple(object, other, order) { // diff
    var index = -1,
        objCriteria = object.criteria,
        othCriteria = other.criteria,
        length = objCriteria.length;

    while (++index < length) {
        var result = baseCompareAscending(objCriteria[index], othCriteria[index]);
        if (result) {
            return order[index] ? -result : result; // diff
        }
    }

    return object.index - other.index;
}

What I like about this approach, using an order-indicating array side-by-side with the criteria array, is that it can easily default to all-ascending when left unspecified/empty. It also doesn't restrict the names of properties like the +/- prefix suggestion does.

The thing I couldn't figure out is correctly modifying sortByAll to "carry" order from the argument list through to e.g. return baseSortBy(result, curryRight(compareMultiple)(order)).

I get tripped up determining whether the iteratee-check area needs any changes, and then also around the call to baseFlatten which might, to continue from the example, end up assigning ['foo', 'bar', false, true] to props if I do something dumb. I don't yet grok things well enough to confidently resolve this.

Anyway, it'd be great to have something like this working for sortByAll. Thanks again.

@jdalton
Copy link
Member

jdalton commented Feb 26, 2015

I'd prefer an options object, maybe dir with a 1 or -1.

@leefernandes
Copy link

Allow props array to optionally contain objects with dir as a value?

var users = [
  { 'user': 'barney', 'age': 36 },
  { 'user': 'fred',   'age': 40 },
  { 'user': 'barney', 'age': 26 },
  { 'user': 'fred',   'age': 30 }
];

// default to standard direction
_.sortByAll(myArray, ['user', 'age'])

or

// define direction
_.sortByAll(myArray, [{user: 1}, {age: -1}])

@jdalton
Copy link
Member

jdalton commented Feb 26, 2015

Crawling keys is not ideal.
It'd be better to have [{ 'key': 'user', 'dir': 1 }, { 'key': 'age', 'dir': -1 }]

@octref
Copy link
Contributor

octref commented Feb 26, 2015

I think for the sake of consistency, this might be better:

_.sortByAll(myArray, ['user', 'age'])
_.sortByAll(myArray, {user: 1, age: -1})

iteratee / predicate style, what do you think @jdalton ?

@jdalton
Copy link
Member

jdalton commented Feb 26, 2015

I'll accept a PR for _.sortByAll(myArray, {user: 1, age: -1}).

@octref
Copy link
Contributor

octref commented Feb 27, 2015

Sounds good. I guess I'll take a stab at that tonight.

@leefernandes
Copy link

Is a sort object an issue since property order of objects are not guaranteed in JavaScript?

Could _.sortByAll(myArray, {user: 1, age: -1}) end up sorting "age" first, where the intention is for the first sort to be of "user"?

4.3.3 Object
An object is a member of the type Object. It is an unordered collection of properties...

http://www.ecma-international.org/publications/files/ECMA-ST-ARCH/ECMA-262,%203rd%20edition,%20December%201999.pdf

@octref
Copy link
Contributor

octref commented Feb 27, 2015

Right. I guess we'd have to type the long
[{ 'key': 'user', 'dir': 1 }, { 'key': 'age', 'dir': -1 }] then :-(.
And I feel 'dir': 'asc' and 'dir': 'desc' looks better, but that's just my opinion.

@leefernandes
Copy link

Maybe to streamline ui implementation consider "reverse: bool" instead of "dir: string".

_.sortByAll(myArray, [{key: "user"}, {key: "age", reverse: true}])
$scope.sort = [{key: 'user'}, {key: 'age', reverse: true}]
<ng-repeat="option in sort">
  <caret ng-click="option.reverse = !option.reverse"></caret>
  <label>{{::option.key}}</label>
</ng-repeat>

@jdalton
Copy link
Member

jdalton commented Feb 27, 2015

Since sorting is ascending by default how about a desc that's either truthy or not.

@leefernandes
Copy link

^^ +1 for desc: true/falsey

@octref
Copy link
Contributor

octref commented Feb 27, 2015

I'm good with desc true/false.
So it'll look like this:

_.sortByAll(users, [
  { 'key': 'user', 'desc': true },
  { 'key': 'age'}, // Default to 'desc': false
  { 'key': 'gender', 'desc': false } // You can also say that explicitly
]);


@jdalton
Copy link
Member

jdalton commented Feb 27, 2015

Keep in mind it would only be needed when different than the default. So regular keys could be used for the common case.

jdalton added a commit that referenced this issue Mar 1, 2015
Implement optional descending order sort in #414.
@NeevShah
Copy link

Hello @jdalton.

Is there any way to pass values with keys in _.sortByAll()? Can I pass boolean as a value?

@jdalton
Copy link
Member

jdalton commented May 12, 2015

Is there any way to pass values with keys in _.sortByAll()? Can I pass boolean as a value?

I'm not sure what you're trying to accomplish. There's _.sortByOrder which allows specifying booleans for sort order direction.

@NeevShah
Copy link

I'm responsible for upgrading lodash to 3.8 version from 2.4.1. Where I came to this code where I thought I can improve code.
So my use case is I want to sort array of object by two properties with values.

Example:

var arr = [{"name" :  "test' , type : "1",isActive :true},{"name" :  "tea', type : "2" , isActive :false},{"name" :  "tea' , type : "1", isActive :true}]; 

Now I want to sort first with {"type" : "1" } and then {isActive : false}.
In 2.4.1 I was doing like (works fine for me)

_.sortBy(_.sortBy(arr, {"type" : 1}), {"isActive" : false}));

Can I use _.sortByAll here? I was trying

_.sortByAll(arr, {"type" : 1, "isActive" : false});

but this didn't work for me.

@jdalton
Copy link
Member

jdalton commented May 12, 2015

@NeevShah Let's move this to gitter.

@lock
Copy link

lock bot commented Jan 20, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

10 participants