-
-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Description
問題
Performance/RangeIncludeは、Range#include?の代わりにRange#cover?を使用してね、という警告をだすCopです。このCopにはいくつか問題があります。
- rangeの値がNumberである場合、パフォーマンスにはほとんど差が出ない。
- 若干
cover?の方が速くはあるっぽい - これはどこにも記述がない
- 若干
- rangeの値がStringである場合、
include?とcover?では異なった結果が得られる。- これはドキュメントには書かれているけど、警告のメッセージには何も書いていない。
そのため、この警告を元にユーザーは間違った修正を行ってしまう可能性があります。
解決方法
range の値の中身によって、警告のメッセージを変えるべきでしょう。
- Numberの場合「ほとんど差がないよ」と書く?
- これは書かなくてもいいかも知れない。
- Stringの場合「挙動が変わるから注意してね」と書く
- もしくは、そもそも警告を出さなくても良いかも知れない。
- 値がリテラルではない場合「Stringでは挙動が変わるよ。」などと注意を書く
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)
Metadata
Metadata
Assignees
Labels
No labels