1
\$\begingroup\$

I'm parsing a file made up of various sections. I have a current_section variable that tracks which portion of the file I'm currently processing and the following check_for_next_section method that returns either the next section based on the contents of the current line or current_section if there are no matches based on the contents of the current line.

This method is getting flagged by rubocop for both 'perceived complexity' and 'cyclomatic complexity'. How can I reduce these?

SECTION_HEADERS = {                                                                                             
  hawaii_1: '"[2] For Hawaii, the following Postal Codes are Zone 44 for Ground',                               
  hawaii_2: '"For Hawaii, the following Postal Codes are Zone 46 for Ground',                                   
  alaska_1: '"[3] For Alaska, the following Postal Codes are Zone 44 for Ground',                               
  alaska_2: '"For Alaska, the following Postal Codes are Zone 46 for Ground'                                    
}.freeze                                                                                                        
                                                                                                                
def check_for_next_section(line, current_section)                                                               
  return :zones if current_section == :preamble && line.starts_with?('Dest. ZIP')                               
  return :hawaii_1 if current_section == :zones && !line.blank? && line.starts_with?(SECTION_HEADERS[:hawaii_1])
  return :hawaii_2 if current_section == :hawaii_1 && line.starts_with?(SECTION_HEADERS[:hawaii_2])             
  return :alaska_1 if current_section == :hawaii_2 && line.starts_with?(SECTION_HEADERS[:alaska_1])             
  return :alaska_2 if current_section == :alaska_1 && line.starts_with?(SECTION_HEADERS[:alaska_2])             
                                                                                                                
  current_section                                                                                               
end
\$\endgroup\$
0
3
\$\begingroup\$

If complexity is getting to high for a method it's usually a good idea to split out helper methods. If you end up to pass in the same parameters to each of these methods, it could also be a good idea to extract a class.

Some solution could be

def check_for_next_section(line, current_section)
  ZoneCheck.new(line, current_section).run
end

class ZoneCheck
  SECTION_HEADERS = {
    hawaii_1: '"[2] For Hawaii, the following Postal Codes are Zone 44 for Ground',
    hawaii_2: '"For Hawaii, the following Postal Codes are Zone 46 for Ground',
    alaska_1: '"[3] For Alaska, the following Postal Codes are Zone 44 for Ground',
    alaska_2: '"For Alaska, the following Postal Codes are Zone 46 for Ground'
  }.freeze

  def initialize(line, current_section)
    @line = line
    @current_section = current_section
  end

  def run
    if zones?
      :zones
    elsif hawaii_1?
      :hawaii_1
    elsif hawaii_2?
      :hawaii_2
    elsif alaska_1?
      :alaska_1
    elsif alaska_2?
      :alaska_2
    else
      current_section
    end   
  end

  private

  attr_reader :line, :current_section

  def zones?
    current_section == :preamble &&
      line.starts_with?('Dest. ZIP')
  end

  def hawaii_1?
    current_section == :zones &&
      !line.blank? &&
      line.starts_with?(SECTION_HEADERS[:hawaii_1])
  end

  def hawaii_2?
    current_section == :hawaii_1 &&
      line.starts_with?(SECTION_HEADERS[:hawaii_2])
  end

  def alaska_1?
    current_section == :hawaii_2 &&
      line.starts_with?(SECTION_HEADERS[:alaska_1])
  end

  def alaska_2?
    current_section == :alaska_1 &&
      line.starts_with?(SECTION_HEADERS[:alaska_2])
  end
end
\$\endgroup\$
2
\$\begingroup\$

I do not understand why !line.blank? is needed. Assuming it is not you could create a second constant:

NEXT_SECTION = {
  { current: :preamble, line_start: 'Dest.' }                    => :zones 
  { current: :zones,    line_start: SECTION_HEADERS[:hawaii_1] } => :hawaii_1
  { current: :hawaii_1, line_start: SECTION_HEADERS[:hawaii_2] } => :hawaii_2,
  { current: :hawaii_2, line_start: SECTION_HEADERS[:alaska_1] } => :alaska_1,
  { current: :alaska_1, line_start: SECTION_HEADERS[:alaska_2] } => :alaska_2
}.freeze

and write your method as follows.

def next_section(current_section, line)
  h, next_section = NEXT_SECTION.find do |h, next_section|
    h == { current: current_section, line_start: line[0, h[:line_start].size] }
  end
  h.nil? ? current_section : next_section
end
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.