HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsMinor

Parsing Day Range with Time in Ruby

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withrangetimeparsingrubyday

Problem

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\sfrom\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 = tim

Solution

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:

-
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

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

-
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)

Context

StackExchange Code Review Q#139366, answer score: 3

Revisions (0)

No revisions yet.