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

Add _.sortByDescending function #1820

Closed
wants to merge 1 commit into from

Conversation

mareek
Copy link

@mareek mareek commented Sep 1, 2014

Add a function to do a stable sorting in reverse order since there is currently no way to this in underscore (_sortBy().reverse() change the order of equal elements).

sortBy has been refactored so that sortBy and sortByDescending share most of their implementation

the_.sortByDescending stably sort a collection in reverse order.
@bjmiller
Copy link

bjmiller commented Sep 2, 2014

I suspect that this will be turned down out-of-hand anyway, but... Would it make more sense to pass an options hash along with plain old sortBy, like {sort: 'descending'}?

@mareek
Copy link
Author

mareek commented Sep 2, 2014

@bjmiller Adding another parameter to sortBy would break existing code (see PR #920 ).

@bjmiller
Copy link

bjmiller commented Sep 2, 2014

Well, let's not do that, then. It was worth a thought.

@jashkenas
Copy link
Owner

For descending sorts, if you don't want to have to reverse — I think that passing an iteratee function is a fine approach. No need for an additional option or separate function.

@mareek
Copy link
Author

mareek commented Sep 2, 2014

I don't see how I can sort on a string property in reverse order by using iteratee function. can you give me an example, please ?

@jashkenas
Copy link
Owner

Of course — you're quite right – I was thinking of using a regular .sort() for this case.

@mareek
Copy link
Author

mareek commented Sep 3, 2014

Unfortunately .sort() isn't stable on every browser when sorting big arrays (I'm looking at you, chrome) so using .sort() won't solve the problem.

@mareek
Copy link
Author

mareek commented Sep 3, 2014

Besides, adding a proper way to sort by descending order would make sort chaining scenario much simpler (instead of relying on tricks like this)

@megawac
Copy link
Collaborator

megawac commented Sep 3, 2014

I think this could be useful but should wait for #1768 and #1751 are resolved

@mareek mareek deleted the sortByDescending branch January 15, 2016 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants