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

Feature: New Style/RedundantSortBlock cop #132

Closed
fatkodima opened this issue Jun 7, 2020 · 1 comment · Fixed by #135
Closed

Feature: New Style/RedundantSortBlock cop #132

fatkodima opened this issue Jun 7, 2020 · 1 comment · Fixed by #135

Comments

@fatkodima
Copy link
Contributor

# bad
array.sort { |a, b| a <=> b }

# good 
array.sort

It is shorter and faster to just use .sort when default implementation is specified explicitly.

If this will be accepted, I would submit a PR. If it is better suited for rubocop-performance - let me know.

Benchmark

# frozen_string_literal: true

require 'bundler/inline'
require 'securerandom'

gemfile(true) do
  gem 'benchmark-ips'
end

ARR10 = 10.times.map { rand(1000) }
ARR1000 = 1000.times.map { rand(1000) }
ARR1000000 = 1000000.times.map { rand(1000) }

ARR_HEXES = 100.times.map { SecureRandom.hex(13) }

puts "======== Hexes ========"

Benchmark.ips do |x|
  x.report("sort")   { ARR_HEXES.sort }
  x.report("sort { ... }")   { ARR_HEXES.sort { |a, b| a <=> b } }
  x.compare!
end

puts "======== 10 ========"

Benchmark.ips do |x|
  x.report("sort")   { ARR10.sort }
  x.report("sort { ... }")  { ARR10.sort { |a, b| a <=> b } }
  x.compare!
end

puts "======== 1000 ========"

Benchmark.ips do |x|
  x.report("sort")   { ARR1000.sort }
  x.report("sort { ... }")  { ARR1000.sort { |a, b| a <=> b } }
  x.compare!
end

puts "======== 1_000_000 ========"

Benchmark.ips do |x|
  x.report("sort")   { ARR1000000.sort }
  x.report("sort { ... }")  { ARR1000000.sort { |a, b| a <=> b } }
  x.compare!
end

Results

======== Hexes ========
Warming up --------------------------------------
                sort     8.389k i/100ms
        sort { ... }     1.599k i/100ms
Calculating -------------------------------------
                sort     83.953k (± 3.5%) i/s -    419.450k in   5.002652s
        sort { ... }     17.233k (± 6.7%) i/s -     86.346k in   5.042224s

Comparison:
                sort:    83953.1 i/s
        sort { ... }:    17232.9 i/s - 4.87x  (± 0.00) slower

======== 10 ========
Warming up --------------------------------------
                sort   138.777k i/100ms
        sort { ... }    40.365k i/100ms
Calculating -------------------------------------
                sort      1.367M (± 8.1%) i/s -      6.800M in   5.018862s
        sort { ... }    396.447k (± 3.2%) i/s -      2.018M in   5.096193s

Comparison:
                sort:  1367357.6 i/s
        sort { ... }:   396447.4 i/s - 3.45x  (± 0.00) slower

======== 1000 ========
Warming up --------------------------------------
                sort   991.000  i/100ms
        sort { ... }   146.000  i/100ms
Calculating -------------------------------------
                sort      9.693k (± 6.2%) i/s -     48.559k in   5.031296s
        sort { ... }      1.416k (± 7.8%) i/s -      7.154k in   5.091314s

Comparison:
                sort:     9693.2 i/s
        sort { ... }:     1415.8 i/s - 6.85x  (± 0.00) slower

======== 1_000_000 ========
Warming up --------------------------------------
                sort     1.000  i/100ms
        sort { ... }     1.000  i/100ms
Calculating -------------------------------------
                sort      9.995  (± 0.0%) i/s -     50.000  in   5.009892s
        sort { ... }      1.431  (± 0.0%) i/s -      8.000  in   5.589687s

Comparison:
                sort:       10.0 i/s
        sort { ... }:        1.4 i/s - 6.98x  (± 0.00) slower
@koic
Copy link
Member

koic commented Jun 8, 2020

There is Performance/CompareWithBlock cop similar to this cop. Maybe it's preferable to be added to rubocop-performance as a new cop.

@koic koic transferred this issue from rubocop/rubocop Jun 8, 2020
@koic koic closed this as completed in #135 Jun 9, 2020
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 a pull request may close this issue.

2 participants