From 6af2355203ac84893bb607ff12681052d0ab00d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20D=C3=ADaz?= Date: Sun, 7 Feb 2021 00:40:36 -0500 Subject: [PATCH] Use native Range#cover? which accepts Range arguments since Ruby 2.6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit: https://github.com/ruby/ruby/commit/9ca738927293df1c7a2a1ed7e2d6cf89527b5438 Discussion: https://bugs.ruby-lang.org/issues/14473 It seems to be compatible with the original ActiveSupport's implementation, at least based on the test suite. It also works faster: ``` Warming up -------------------------------------- Ruby's Range#cover? 1.196M i/100ms ActiveSupport's Range#cover? 396.369k i/100ms Calculating ------------------------------------- Ruby's Range#cover? 11.889M (± 1.7%) i/s - 59.820M in 5.033066s ActiveSupport's Range#cover? 3.951M (± 1.2%) i/s - 19.818M in 5.017176s Comparison: Ruby's Range#cover?: 11888979.3 i/s ActiveSupport's Range#cover?: 3950671.0 i/s - 3.01x (± 0.00) slower ``` Benchmark script: ```ruby require "minitest/autorun" require "benchmark/ips" module ActiveSupportRange def active_support_cover?(value) if value.is_a?(::Range) is_backwards_op = value.exclude_end? ? :>= : :> return false if value.begin && value.end && value.begin.public_send(is_backwards_op, value.end) # 1...10 covers 1..9 but it does not cover 1..10. # 1..10 covers 1...11 but it does not cover 1...12. operator = exclude_end? && !value.exclude_end? ? :< : :<= value_max = !exclude_end? && value.exclude_end? ? value.max : value.last cover?(value.first) && (self.end.nil? || value_max.public_send(operator, last)) else cover? end end end class BugTest < Minitest::Test def test_range_cover Range.prepend(ActiveSupportRange) range = (1..10000) Benchmark.ips do |x| x.report("Ruby's Range#cover?") do range.cover?((100..20)) end x.report("ActiveSupport's Range#cover?") do range.active_support_cover?((100..20)) end x.compare! end end end ``` --- .../core_ext/range/compare_range.rb | 25 ---------------- activesupport/test/core_ext/range_ext_test.rb | 29 ------------------- .../source/active_support_core_extensions.md | 9 ++---- 3 files changed, 2 insertions(+), 61 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/range/compare_range.rb b/activesupport/lib/active_support/core_ext/range/compare_range.rb index c6d5b77e8e..affbbebb42 100644 --- a/activesupport/lib/active_support/core_ext/range/compare_range.rb +++ b/activesupport/lib/active_support/core_ext/range/compare_range.rb @@ -51,31 +51,6 @@ module ActiveSupport super end end - - # Extends the default Range#cover? to support range comparisons. - # (1..5).cover?(1..5) # => true - # (1..5).cover?(2..3) # => true - # (1..5).cover?(1...6) # => true - # (1..5).cover?(2..6) # => false - # - # The native Range#cover? behavior is untouched. - # ('a'..'f').cover?('c') # => true - # (5..9).cover?(11) # => false - # - # The given range must be fully bounded, with both start and end. - def cover?(value) - if value.is_a?(::Range) - is_backwards_op = value.exclude_end? ? :>= : :> - return false if value.begin && value.end && value.begin.public_send(is_backwards_op, value.end) - # 1...10 covers 1..9 but it does not cover 1..10. - # 1..10 covers 1...11 but it does not cover 1...12. - operator = exclude_end? && !value.exclude_end? ? :< : :<= - value_max = !exclude_end? && value.exclude_end? ? value.max : value.last - super(value.first) && (self.end.nil? || value_max.public_send(operator, last)) - else - super - end - end end end diff --git a/activesupport/test/core_ext/range_ext_test.rb b/activesupport/test/core_ext/range_ext_test.rb index 59a5b8897a..9a7fb224b7 100644 --- a/activesupport/test/core_ext/range_ext_test.rb +++ b/activesupport/test/core_ext/range_ext_test.rb @@ -151,35 +151,6 @@ class RangeTest < ActiveSupport::TestCase assert range.method(:include?) != range.method(:cover?) end - def test_should_cover_other_with_exclusive_end - assert((1..10).cover?(1...11)) - end - - def test_cover_returns_false_for_backwards - assert_not((1..10).cover?(5..3)) - end - - # Match quirky plain-Ruby behavior - def test_cover_returns_false_for_empty_exclusive_end - assert_not((1..5).cover?(3...3)) - end - - def test_should_cover_range_with_endless_range - assert((1..).cover?(2..4)) - end - - def test_should_not_cover_range_with_endless_range - assert_not((1..).cover?(0..4)) - end - - def test_should_cover_range_with_beginless_range - assert((..2).cover?(-1..1)) - end - - def test_should_not_cover_range_with_beginless_range - assert_not((..2).cover?(-1..3)) - end - def test_overlaps_on_time time_range_1 = Time.utc(2005, 12, 10, 15, 30)..Time.utc(2005, 12, 10, 17, 30) time_range_2 = Time.utc(2005, 12, 10, 17, 00)..Time.utc(2005, 12, 10, 18, 00) diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index c107894a0c..5bb502bab6 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -3159,9 +3159,9 @@ As the example depicts, the `:db` format generates a `BETWEEN` SQL clause. That NOTE: Defined in `active_support/core_ext/range/conversions.rb`. -### `===`, `include?`, and `cover?` +### `===` and `include?` -The methods `Range#===`, `Range#include?`, and `Range#cover?` say whether some value falls between the ends of a given instance: +The methods `Range#===` and `Range#include?` say whether some value falls between the ends of a given instance: ```ruby (2..3).include?(Math::E) # => true @@ -3179,11 +3179,6 @@ Active Support extends these methods so that the argument may be another range i (1..10).include?(0..7) # => false (1..10).include?(3..11) # => false (1...9).include?(3..9) # => false - -(1..10).cover?(3..7) # => true -(1..10).cover?(0..7) # => false -(1..10).cover?(3..11) # => false -(1...9).cover?(3..9) # => false ``` NOTE: Defined in `active_support/core_ext/range/compare_range.rb`.