# Adding minutes to a time value in Ruby

I recently submitted a code-challenge for an interview according to the following guidelines:

Write a function or method that accepts two mandatory arguments and returns a result.

Requirements:

• Use Ruby or another language of your choosing, and write production quality code.
• Your function or method should not use any date/time library.
• The first argument is a time value in the form of a string with the following format: [H]H:MM {AM|PM}
• The second argument is the number of minutes, as an integer value
• The return value of the function should be a string of the same format as the first argument, which is a result of adding the minutes to the time.
• For example, add_minutes('9:13 AM', 10) would return 9:23 AM

• We just want to see how you code, so this exercise is not meant to be too hard or take too long.
• If you spend an hour on this, stop coding and finalize by adding some notes about what you would do if you had more time.

Although my Ruby proficiency has somewhat atrophied due to primarily working with php for the last 8 months, I was able to produce the following well-within one-hour of beginning the challenge:

# FIRST ATTEMPT

# require 'time'

# def flux_capacitor(time,mins)
#   the_time = Time.parse(time.to_s)
#   mins = the_time + 10*60
# end

# FINAL REFACTOR

def flux_capacitor(time,mins)
the_time = time.scan(/\d/).join('').to_i
meridian = time.scan(/(A|M|P)/).join('')
new_time = the_time + mins
back_in_time = new_time.to_s.insert(-3,':') + " #{meridian}"
end

puts flux_capacitor("9:13 AM",10)
puts flux_capacitor("9:13 PM",10)
p flux_capacitor("10:13 PM",10).class

# DOX

# http://ruby-doc.org/core-2.2.3/Time.html
# https://www.ruby-forum.com/topic/125709
# https://www.ruby-forum.com/topic/104359
# http://apidock.com/ruby/String/insert
# http://rubular.com/


Simply-stated, is there a better approach? I've asked others and they think my solution is fairly clean. Obviously, I know my solution could benefit with a conditional statement for edge cases (e.g. flux_capacitor("9:50 AM", 15)).

More testing is required:

flux_capacitor("11:59 AM", 62)
=> "12:21 AM"


Definitely test some corner cases.

## Updated Thoughts

I think you should completely rethink the way you are handling time. Use a regex to validate that time is a valid string, then simply use string.split to get each portion of the time: hours, minutes, and AM/PM.

Next you should convert those into a quantity of minutes:

hours += 12 if meridian == 'PM'
total_minutes = hours*60+minutes+mins


Next I would recalculate the time as a 24-hour time (which goes up to 23 hours, 59 minutes):

# this is integer math so the decimal portion will be discarded
# adjusted_minutes may be greater than 23, but adjusted_minutes should always be less than 60

# modulus 24 converts adjusted_hours into 24-hour time


Now convert to 12 hour time if necessary and determine if this is AM or PM

# the added mins may have been several days worth, so adjusted_hours might be very large.

meridian = "PM"
else
meridian = "AM"
end


Finally return a string of the new time:

# zero pad the minutes, as this is expected for a time


# Final Solution

Since a solution that reliably passes all test cases has yet to be put forth...

def flux_capacitor(time, mins)
#check if time in valid format
time_match = time.strip.match /^(12|11|10|0?\d):([012345]\d)\s+(AM|PM)/

#throw error on invalid time
raise(ArgumentError, "Invalid time: #{time.strip}") if not time_match

#calculate new time
strhours, strminutes, meridian = time_match.captures
hours = (meridian == "AM" ? strhours.to_i : strhours.to_i + 12)
total_minutes = hours * 60 + strminutes.to_i + mins
total_minutes = total_minutes % (24*60) # we only want the minutes that fit within a day
meridian = "PM"
else
meridian = "AM"
end

end


Testing:

[ "11:13 PM",
"13:09 PM", #invalid!
"1:59 AM",
"04:49 PM",
"4:79 PM" #invalid!
].each do |time|
begin
puts flux_capacitor(time, 0)
rescue
puts $! end end puts "----------" puts "#{flux_capacitor("11:13 PM", 10)}, expected: 11:23 PM" puts "#{flux_capacitor("11:13 PM", 12*60)}, expected 11:13 AM" puts "#{flux_capacitor("11:13 PM", 24*60)}, expected 11:13 PM" puts "#{flux_capacitor("11:13 PM", 24*60 + 1)}, expected 11:14 PM" puts "#{flux_capacitor("11:59 AM", 62)}, expected 1:01 PM" puts "#{flux_capacitor("11:59 PM", 62)}, expected 1:01 AM"  • @zack, agreed, my solution does indeed require additional constraints to satisfy edge-cases. Thank you. – alexanderjsingleton Feb 29 '16 at 19:17 • I debated it as a comment, but given that the code has some fundamental issues, I made it an answer. He's going to need to ask a new question for the revised code in any case. – Zack Mar 1 '16 at 1:06 • @alexanderjsingleton I've added some thought on an alternative implementation for consideration – Zack Mar 1 '16 at 2:47 • I'd be tempted to use the magic $1, $2 globals after the regex validation - eliminates the need for split. E.g. puts "#{$1}h #{$2}m" if "23:42" =~ /(\d+):(\d\d)/ – Flambino Mar 1 '16 at 9:52 • @alexanderjsingleton Your code on git fails with flux_capacitor("11:59 PM", 62). It reports PM while the meridian should be blank or AM – Zack Mar 2 '16 at 16:32 Some notes: • time.scan(/\d/): That's too general, better use a regexp that mimics exactly the input you should have. I'd use time.match, in any case. • Why /(A|M|P)/ instead of (AM|PM)? • new_time.to_s.insert(-3,':'). Algorithms are cleaner with no in-place updates, create new variables. • The edge cases (carry values of hours/meridian) is the core of this algorithm, IMO, you cannot left it out. • It's not specified what to do for invalid inputs, but a "production-quality" code should consider it somehow, your code raises uncontrolled exceptions. Two options: return nil or raise an exception with a nice error message. I'd write: def flux_capacitor(strtime, minutes_diff) time_match = strtime.match(/^(\d?\d):(\d\d) (AM|PM)$/)
if time_match
strhour, strminute, meridian = time_match.captures
hours_carry, new_minute = (strminute.to_i  + minutes_diff).divmod(60)
new_hour = (strhour.to_i - 1 + hours_carry) % 12 + 1
meridian_crosses = ((strhour.to_i % 12) + hours_carry) / 12
new_meridian = (meridian.eql?("AM") == meridian_crosses.even?) ? "AM" : "PM"
"%d:%02d %s" % [new_hour, new_minute, new_meridian]
else
raise(ArgumentError, "Invalid format: expected [H]H:MM {AM|PM}, got #{strtime}")
end
end

• Thank you, @tokland! I'll follow-up after experimentation. – alexanderjsingleton Mar 1 '16 at 14:53

I reverted back to my MVP, which is a mash-up derived from the above submissions and my initial solution- now accounting for edge-cases, sans error-flags. If submitting refactors as an answer is a breach of SE/Code Review protocol, I will happily repost another question for review. Thank you.

def flux_capacitor(time,mins)
time_match = time.match(/^(\d?\d):(\d\d) (AM|PM)$/) strhours, strminutes, meridian = time_match.captures if time_match military_clock = (strhours.to_i * 60 + strminutes.to_i) + mins adjusted_military_clock_hours = military_clock / 60 adjusted_military_clock_minutes = military_clock - (60 * adjusted_military_clock_hours) if adjusted_military_clock_hours > 12 meridian = "PM" adjusted_military_clock_hours -= 12 else end end return "%d:%02d %s" % [adjusted_military_clock_hours, adjusted_military_clock_minutes, meridian] end puts flux_capacitor("9:13 AM",10) puts flux_capacitor("9:13 PM",10) p flux_capacitor("10:13 AM",10).class puts flux_capacitor("11:59 AM", 62)  • @flambino, I couldn't quite effect your "magical" suggestion re: $1, \$2 globals after the regex validation. Any chance I can persuade you to submit a solution or at least edit my submission? I'd sincerely appreciate it- thank you! – alexanderjsingleton Mar 1 '16 at 18:31
• If the regex match fails, you should return an error message. This code fails on an invalid input, like flux_capacitor("9:xy AM",10) => "undefined method captures' for nil:NilClass". Also note that it currently accepts inputs like "9:97 AMP" and returns an answer. – Zack Mar 1 '16 at 18:40
• Thanks, @Zack- I focused the meridian regex; now I'm working on that argument-error message. – alexanderjsingleton Mar 2 '16 at 2:10
• better to use a guard clause as the first line raise ArgumentError unless time =~ /(\d+):(\d+) AM|PM/` instead of adding an unsightly layer of indentation to the entire method body – Jonah Mar 2 '16 at 3:32