Skip to content

Conversation

sshaw
Copy link
Contributor

@sshaw sshaw commented May 26, 2019

No description provided.

@semaperepelitsa
Copy link

Should really be compared with sort_by{ |a| -a }or sort_by(&:-@)

@ixti
Copy link
Collaborator

ixti commented Jul 31, 2020

arr.reverse.sort simply makes no sense. Array#reverse returns reverse-ordered copy of an array.

Comparing sort_by { |a| -a } to sort_by(&:-@) was already covered in:

https://github.com/JuanitoFatas/fast-ruby/blob/master/code/enumerable/sort-vs-sort_by.rb

Comparing sort { |a, b| b <=> a } to sort_by(&:-@) is also pointless as it's already covered by the above.

@ParadoxV5
Copy link

Did you mean, sort.reverse!?

@sshaw
Copy link
Contributor Author

sshaw commented Aug 1, 2020

Should really be compared with sort_by{ |a| -a }or sort_by(&:-@)

@semaperepelitsa agreed. Updated.

arr.reverse.sort simply makes no sense.

@ixti typo, should be arr.sort.reverse. Fixed.

Comparing sort_by { |a| -a } to sort_by(&:-@) was already covered in:

@ixti yes I know, I wrote it. This PR differs by 50%.

Did you mean, sort.reverse!?

@ParadoxV5 yes updated (though no !)

@sshaw
Copy link
Contributor Author

sshaw commented Aug 1, 2020

Though technically this is comparing Array#sort etc... not Enumerable. Updated.

@sshaw sshaw changed the title Add reverse.sort vs sort with block Add Array#sort.reverse vs Array#sort_by with block Aug 1, 2020
@ParadoxV5
Copy link

ParadoxV5 commented Aug 1, 2020

though no !

@sshaw Not sure if the sort! variant is faster or not because several other languages (including our rival Python) do their sorting in-place (modifying the original). (reverse and sort each creates a new array with reversed contents.)

Also, should compare with _2 <=> _1 because Numeric happens to have negation functionality (@-), but not guaranteed any Comparable duck-type with <=>, such as Strings (@- doesn’t negate) and Arrays (@- isn’t defined). (P.P.S Though we can include Numeric reverse-sorting separately as a special case.)

P.S. @ixti The above file does not sort reversedly.

@ixti
Copy link
Collaborator

ixti commented Aug 4, 2020

I still think this comparison is weird and very niche - it has meaning only when the object is an Array of Numbers. In which case the bet variant will be: arr.sort.tap(&:reverse!), in other cases it will either fail or will give unexpected result.

@ParadoxV5 I know that the sort-vs-sort_by.rb has no reverse sorting.

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sshaw Thanks for submitting this! I think it adds value to the project.

@etagwerker etagwerker merged commit 7c87d13 into fastruby:main Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants