How would you refactor this method to reduce the ABC size to less than 15?

Question

require 'active_support'
require 'active_support/core_ext'

def subtract_two_ranges(range_a, range_b)
  return [range_a] unless range_a.overlaps?(range_b)
  [
    ((range_a.begin..(range_b.begin - 1)) if range_a.begin < range_b.begin),
    (((range_b.end + 1)..range_a.end)     if range_b.end   < range_a.end)
  ].compact
end

Answer 1

I have presented two methods below. Both are pure-Ruby and operate on arbitrary finite ranges.¹. The first is the more efficient, but more complex.

Efficient but complex

Code

def subtract_range(r1, r2)
  return [r1] if r2.end   < r1.begin  || r2.begin > r1.end
  return []   if r2.begin <= r1.begin && r2.end  >= r1.end
  if r2.begin <= r1.begin
    r2.end < r1.begin ? [r1] : [r2.end.succ..r1.end]
  else # r1.begin < r2.begin <= r1.end
    e = pred(r1.begin, r2.begin)
    r2.end >= r1.end ? [(r1.begin)..e] :
      [r1.begin..e, (r2.end.succ)..r1.end]
  end
end

def pred(start_at, e)
  (return e-1) if e.kind_of? Numeric
  loop do
    break start_at if start_at.succ == e
    start_at = start_at.succ
  end
end

All elements of a range (regardless of their class) have a method succ (e.g., String#succ), but in general do not have a comparable predecessor method. Numeric classes are the exception: the predecessor of n is n-1. For non-numeric elements one must compute the predecessor value by starting at some known smaller value and apply succ successively until the predecessor value is found.

Examples

subtract_range 15..25, 20..30  #=> [15..19]
subtract_range 15..25, 10..20  #=> [21..25]
subtract_range 15..25, 17..23  #=> [15..16, 24..25]
subtract_range 15..25,  5..10  #=> [15..25]
subtract_range 15..25, 30..35  #=> [15..25]
subtract_range 15..25, 25..30  #=> [15..24]
subtract_range 15..25, 16..30  #=> [15..15]
subtract_range 15..25, 10..30  #=> []
subtract_range 'd'..'j', 'g'..'m'  #=> ["d".."f"]
subtract_range 'd'..'j', 'f'..'h'  #=> ["d".."e", "i".."j"]

Less efficient but simpler

Code

def subtract_range(r1, r2)
  arr = r1.to_a - r2.to_a
  return [] if arr.empty?
  return [arr.first..arr.first] if arr.size == 1
  arr.slice_when { |m,n| m.succ < n }.
      map { |a| a.size == 1 ? a.first..a.first : a.first..a[-1] }
end

Examples

This method produces the same return values for the examples given above.

Explanation

Suppose (from the last example)

r1 = 'd'..'j'
r2 = 'f'..'h'

Then

arr = r1.to_a - r2.to_a
  #=> ["d", "e", "f", "g", "h", "i", "j"] - ["f", "g", "h"] 
  #=> ["d", "e", "i", "j"]
arr.empty?
  #=> false (so do not return [])
arr.size == 1
  #=> false (so do not return [arr.first..arr.first])
enum = arr.slice_when { |m,n| m.succ < n }
  #=> #<Enumerator: #<Enumerator::Generator:0x00000001d2c228>:each>

We can convert enum to an array to see the elements that will be passed to map.

enum.to_a
  #=> [["d", "e"], ["i", "j"]]

Lastly,

enum.map { |a| a.size == 1 ? a.first..a.first : a.first..a[-1] }
  #=> ["d".."e", "i".."j"]

^{1 2.0..6.0 - 4.0..8.0 #=> 2.0...4.0, is not a problem (note the three dots). However, 2.0..4.0 - 1.0..3.0is a problem, as the resulting range,3.0..4.0, excluding 3.0, cannot be represented as a Range instance.}

Answer 2

ABC size typically indicates that your method is too complicated, hard to read and, probably, breaks SRP.

1. It should be a method of a Range.
2. Move expressions in array to its own private methods.
3. Use refinements to avoid monkey-patching a base class.

Try smth like this.

require 'active_support'
require 'active_support/core_ext'

class Range
  def -(other)
    return [self] unless overlaps?(other)
    [substract_lower_bound(other), substract_upper_bound(other)].compact
  end

  private

  def substract_lower_bound(other)
    first..(other.begin - 1) if first < other.first
  end

  def substract_upper_bound(other)
   (other.last + 1)..last if other.last < last
  end
end