rubocop / rubocop-jp

A place for RuboCop discussions in Japanese
55 stars 2 forks source link

`Performance/RangeInclude`を色々直したい。 #20

Open pocke opened 7 years ago

pocke commented 7 years ago

問題

Performance/RangeIncludeは、Range#include?の代わりにRange#cover?を使用してね、という警告をだすCopです。このCopにはいくつか問題があります。

そのため、この警告を元にユーザーは間違った修正を行ってしまう可能性があります。

解決方法

range の値の中身によって、警告のメッセージを変えるべきでしょう。

Note

これだけ聞くとこのCopは無意味なのでは、と思うかも知れませんが、例えばTimeのrangeの場合などはそこそこパフォーマンスに差が出てくるので意味があると言えるでしょう。

require 'benchmark'
Benchmark.bm(20) do |x|
  time1 = Time.new(1994, 2, 2)
  time2 = Time.new(2017, 11, 13)
  time3 = Time.new(1990, 11, 24)
  time4 = Time.new(2020, 8, 16)

  range = 1..100000
  range_time = time1..time2

  x.report('include? Number') do
    10000000.times do
      range.include?(-1)
      range.include?(1)
      range.include?(5667)
      range.include?(100000000)
    end
  end

  x.report('cover? Number') do
    10000000.times do
      range.cover?(-1)
      range.cover?(1)
      range.cover?(5667)
      range.cover?(100000000)
    end
  end

  x.report('include? Time') do
    10000000.times do
      range_time.include?(time1)
      range_time.include?(time3)
      range_time.include?(time4)
    end
  end

  x.report('cover? number') do
    10000000.times do
      range_time.cover?(time1)
      range_time.cover?(time3)
      range_time.cover?(time4)
    end
  end
end
                           user     system      total        real
include? Number        2.640000   0.000000   2.640000 (  2.639529)
cover? Number          2.590000   0.000000   2.590000 (  2.585174)
include? Time          4.260000   0.000000   4.260000 (  4.262342)
cover? number          3.770000   0.000000   3.770000 (  3.770333)