F0f3934c3feaecd1897dcf1cf55769fa

I needed to detect if the current time is inside a time window. How can this be improved?

Cheers z0mbix

#!/usr/bin/env ruby
#
# Define window start/stop times
$start="1800"
$stop="0500"

# Check start/stop times are valid
def valid_times?(start, stop)
  if (start[0,2].to_i.between?(0, 23) && start[2,4].to_i.between?(0, 59)) &&
    (stop[0,2].to_i.between?(0, 23) && stop[2,4].to_i.between?(0, 59))
    return true
  else
    return false
  end
end

# Check that time is within the start/stop time window
def within_time_window?(time, start, stop)
  t = time.to_i
  # Range on same day
  if start.to_i < stop.to_i
    if (start.to_i > t) || (stop.to_i < t)
      return false
    end
  # Range over night
  else
    if (start.to_i > t) && (stop.to_i < t)
      return false
    end
  end
  return true
end

if valid_times?($start, $stop)
  time = Time.now.strftime("%H%M")

  if within_time_window?(time, $start, $stop)
    puts "Inside time window"
  else
    puts "Not inside time window"
  end
else
  puts "Start/Stop times not valid!"
end

Refactorings

No refactoring yet !

A3897deec9b1dbc6def9f5dfcb58bb60

spaghetticode

March 16, 2011, March 16, 2011 00:47, permalink

No rating. Login to rate!

I would go with a more object oriented approach: it will make easier to read, use, mantain and extend the code.

class TimeWindow  
  def initialize(start_time, end_time)
    @start_time = WindowTime.new(start_time)
    @end_time   = WindowTime.new(end_time)
    raise ArgumentError, 'Times are invalid' unless valid_times?
  end
  
  def include?(time)
    time = WindowTime.new(time)
    if @start_time < @end_time
      compared_time >= @start_time && time <= @end_time
    else
      time >= @start_time || time <= @end_time
    end
  end
  
  private
  
  def valid_times?
    @start_time.valid? && @end_time.valid?
  end
end

class WindowTime
  include Comparable
  
  attr_reader :time
  
  def initialize(time)
    @time = time
  end
  
  def hours
    time[0,2].to_i
  end
  
  def minutes
    time[2,4].to_i
  end
  
  def valid?
    (0..23).include?(hours) && (0..59).include?(minutes)
  end
  
  def <=>(other)
    time.to_i <=> other.time.to_i
  end
end

window = TimeWindow.new('1800', '0500')

puts window.include?(Time.now.strftime('%H%M'))
A3897deec9b1dbc6def9f5dfcb58bb60

spaghetticode

March 16, 2011, March 16, 2011 00:49, permalink

No rating. Login to rate!

sorry, somehow my refactoring was posted twice

Your refactoring





Format Copy from initial code

or Cancel