Parsing Day Range with Time in Ruby

Question

I need to shorten/simplify this code:

require "time"
  def parse(timings)
    validdays = ["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"]
    hours =  {}
    if timings.include?("Open everyday from")
      validdays.each do |day|
        open_close = timings.gsub(/.*everyday\s*from\s*(.*).*/,'\1')
        save_day_array(open_close,day,hours)
      end
    else
      if !timings.start_with?('Mon')
        timings.gsub!(/.*(Mon.*)/,'\1')
      end
      timings.slice! "|"
      ap timings
      normalizie_days = {
        "Sun" => "Sunday",
        "Mon" => "Monday",
        "Tue" => "Tuesday",
        "Wed" => "Wednesday",
        "Thu" => "Thursday",
        "Fri" => "Friday",
        "Sat" => "Saturday"}

      #the following Step scans for the Short Days(Mon Tue .. ) and normalizie it (Monday, Tuesday ...)

      if timings.scan(/\b[a-zA-Z]{3}\b/).length > 0
        timings.scan(/\b[a-zA-Z]{3}\b/).each do |d|
          timings.gsub!(/#{d}/,"#{normalizie_days[d]}")
        end
      end

     # Adds all the Matching days to match days array

      match_days = []
      validdays.each do |day|
        if timings.match(day)
          match_days << day
        end
      end

      if match_days.length == 2
       #eg : Monday - Sunday : 10:00am-04:00pm
        if timings.match(/^[a-zA-Z]+\s?-\s?[a-zA-Z]+:.*/)
          validdays.each do |day|
            open_close = timings.gsub(/.*Sunday:(.*)/,'\1')
            save_day_array(open_close,day,hours)
          end
        end
      elsif match_days.length == 3
        validdays.each do |day|
          if timings.match(/^[a-zA-Z]+\s+?-\s+?[a-zA-Z]+:.*\s+?[a-zA-Z]+:?.*/)
            open_close = []
            #eg: Monday - Saturday: 10:00am-7:00pm Sunday: 10:00am - 5:00pm
            if match_days.include?("Sunday")
              if day == "Sunday" && timings.match(day)
                open_close = timings.gsub(/.*Sunday:?(.*)/,'\1')
              else
                open_close = timings.gsub(/^[a-zA-Z]+\s+?-\s+?[a-zA-Z]+:?\s+?(.*)\s+Sunday.*/,'\1')
              end
            else
              hours["sunday"] = [{open: "closed",close: "closed"}]
              if day == "Saturday" && timings.match(day)
                open_close = timings.gsub(/.*Saturday:?\s?\-?\s?(.*)/,'\1')
              else
                open_close = timings.gsub(/^[a-zA-Z]+\s?-\s+?[a-zA-Z]+:?\s?(.*)\s?Saturday.*/,'\1')
              end
            end
            save_day_array(open_close,day,hours)
          end
        end
      elsif match_days.length == 4
       #eg: Sunday - Tuesday: 9:00am-5:00pm Wednesday-Saturday: 9:00am - 7:00pm
        validdays.each do |day|
          if timings.match(/^[a-zA-Z]+\s?\-?\s?[a-zA-Z]+:?.*\s+?[a-zA-Z]+:?.*/)
            open_close = []
            if (day == "Sunday") && timings.match(day)
              open_close = timings.gsub(/.*Sunday:?(.*)/,'\1')
            elsif (day == "Saturday") && timings.match(/.*Saturday\s?\&?\s?Sunday.*|.*Saturday\s?\-?\s?Sunday.*/)
              open_close = timings.gsub(/.*Sunday:?(.*)/,'\1')
            elsif (day == "Saturday") && timings.match(/.*Saturday.*Sunday.*/)
              open_close = timings.gsub(/.*Saturday(.*)Sunday.*/,'\1')
            elsif (day == "Saturday") && timings.match(day)
              open_close = timings.gsub(/.*Sunday:?(.*)/,'\1')
            else
              open_close = timings.gsub(/^[a-zA-Z]+\s?-\s?[a-zA-Z]+:?\s+?(.*)\s+Sunday.*/,'\1')
            end
            save_day_array(open_close,day,hours)
          elsif timings.match(/^[a-zA-Z]+\s?-\s?[a-zA-Z]+:?.*\s+?[a-zA-Z]+:?.*/)
          end
        end
      elsif match_days.length == 7
       #eg: Monday: 10:00am-5:00pm Tuesday: 10:00am-5:00pm Wednesday: 10:00am-5:00pm Thrusday: 10:00am-5:00pm Friday: 10:00am-9:00pm Saturday: 10:00am-9:00pm Sunday: Closed
        match_days.each_with_index do |day, index|
          regex = /.*#{day}(.*)#{match_days[index+1]}.*/
          open_close = timings.gsub(regex,'\1')
          save_day_array(open_close,day,hours)
        end
      else
        ap timings
      end
    end
    return hours
  end

save_day_array to save the day time to hours array created to avoid code repetition

def save_day_array(open_close,day,hours)
    if !open_close.nil?
      open_close = open_close.split("-")
    end
    parsed_open = Time.parse("#{open_close[0]}").strftime("%H:%M")
    parsed_closed = Time.parse("#{open_close[1]}").strftime("%H:%M")
     if !parsed_open.nil? && !parsed_closed.nil?
        hours[day.downcase] = {opens_at: parsed_open,closes_at: parsed_closed}
        else
        hours[day.downcase] = {opens_at: "closed",closes_at: "closed" }
     end
  end

The following strings need to be passed to get the below output:

"Monday - Friday 10:00am-7:00pm Saturday: 10:00am-4:00pm Sunday: 11:00am-4:00pm" 
"Monday - Friday 10 am - 6 pm Saturday 10 am - 5 pm Sunday 11 am - 5 pm" 
"Mon - Sun: 9:00am-7:00pm"

Output :

{
   "sunday": {"opens_at": 11:00, "closes_at": 16:00},
   "monday": {"opens_at": 10:00, "closes_at": 16:00},
   "tuesday": {"opens_at": 10:00, "closes_at": 16:00},
   "wednesday": {"opens_at": 10:00, "closes_at": 16:00},
   "thrusday": {"opens_at": 10:00, "closes_at": 16:00},
   "friday": {"opens_at": 10:00, "closes_at": 16:00},
   "saturday": {"opens_at": 10:00, "closes_at": 16:00}
}

I get the output, but I need to clean up and simplify the code.

Answer 1

OK, this first bit is going to be a bit of opinion, but you seriously need to lower your cyclomatic complexity in the parse(timings) method for a couple different reasons and in a couple of ways:

1. You are doing way way too much in a single method, generally people say that a single method should be at most 20 lines, Any longer and it should be separated into shorter methods which each do a short, but specific operation to the data and return the results so another method can do further processing

2. Using regular expressions is probably a bit overkill for this, and using so many will quickly decrease the maintainability for the entire functionality segment.

3. You are nesting way too much control flow recursively in the method, this is going to make it extremely difficult down the road to easily understand how it works and be able to make changes to it or maintain it any. Generally if I get ~3 control flow statements (if, else, while, for, ...) deep, I extract parts of the logic into new methods.

If I were asked to build this functionality, Here is how I would approach it:

Firstly, you are given a beautiful string that holds all the information you need, take it and slice it into a list separated by spaces. Everything else in the list should still be in order otherwise there can be problems.

Secondly, I would iterate over that list looking at each element to create a state machine built out of only what is provided.

For example, the first element in the list split from the string "Monday - Friday 10:00am-7:00pm Saturday: 10:00am-4:00pm Sunday: 11:00am-4:00pm"

Would be "Monday" which means it can either be the start of a range of days, or it can be business hours for Monday only.

We can tell which by looking at the next element a dash - indicating it is a range ending in the next element Friday.

The end of the range has now been found so we expect a time range to follow, and sure enough 10:00am-7:00pm The hours! Rinse and repeat in this way until the end of the string, and you will have extracted AND understood all the data.

Get that working on the simple case, then add features like "Open Every Day From" and it will be a ton easier to understand, read, and maintain. (not to mention probably significantly faster too)

Answer 2

The Code after the changes to original Question included the day_range.rb file, this contains all the day constants.

require "time"
require "./day_range.rb"
class TimeParser
  def parse(timing)
    #eg: string in format 
    #Before Normalize::  "Monday - Saturday: 9:00am-7:30pm Sunday: 9:00am - 6:00pm"
    @dr = DayRange.new()

    validdays = @dr.range_to_a('Sun', 'Sat')
    #if the timing string contains "Open everyday from" it will iterate over all validays array and will return the json output
    if timing.include?("Open everyday from")
      hours =  {}
      validdays.each do |day|
        open_close = timing.gsub(/.*everyday\s*from\s*(.*).*/,'\1')
        save_day_array(open_close.downcase,day,hours)
      end
      return hours
    end

    #Normalize the String Begin
    timing.gsub!(/\s?-\s?/,'-')
    timing.gsub!(/\s?am\s?/i,'am').gsub!(/\s?pm/i,'pm')
    timing.gsub!(/day\s?:\s|day\s+/i,'day|')
    #Normalize the String End

    #eg: string in format 
    #After Normalize::  "Monday-Saturday|9:00am-7:30pm Sunday|9:00am-6:00pm"

    timings = timing.downcase.split(" ") #now its easy to split on space
    hours =  {} 
    timings.each do |t|
      day_range_with_time = t.split("|")
      day_range = day_range_with_time.first.split("-")
      if day_range.length > 1
        start_day = day_range[0]
        stop_day = day_range[1].gsub(/^(.*day).*/,'\1')
        week_days = @dr.range_to_a(start_day, stop_day) #gets day range eg: Monday - Friday will generate the array from Monday to Friday
        bussiness_hours = day_range_with_time[1]
        if bussiness_hours.nil?
          bussiness_hours = t.gsub(/.*#{stop_day}(.*)/i,'\1')
        end
        week_days.each do |dr|
          save_day_array(bussiness_hours,dr,hours)
        end
      else
        save_day_array(day_range_with_time[1],day_range_with_time[0],hours)
      end
    end
    return hours
  end

  def save_day_array(open_close,day,hours)
    parsed_open,parsed_closed = "closed", "closed"
    open_close = open_close.split("-") if !open_close.nil? && !open_close.include?("closed")
    if !open_close.nil? && !open_close.include?("closed")
      parsed_open = Time.parse("#{open_close[0]}").strftime("%H:%M")
      parsed_closed = Time.parse("#{open_close[1]}").strftime("%H:%M")
    end

    hours[day.downcase] = {opens_at: parsed_open,closes_at: parsed_closed}
  end 
end