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

Check if a store is currently open

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

Problem

This is the dirty bastard of my current code block, and I'm trying to find a way to improve this code for increased readability, but also functionality.

The method checks if the store is currently open. Yet on our site we close the sites 30 minutes prior to their real closing time. Issues arise mostly because all of our stores are open past midnight.

So this function is supposed to figure out if a store is open, taking into account that we close it 30 minutes before it's actual close time, and the fact that if it closes 12:10AM the next day, we have to close it 11:40PM the prior day.

def open?(current_time: Time.now.change(sec: 0))
  current_business_hour = business_hours.where('week_day = ? AND open_at = ?', current_time.wday, current_time, current_time).first
  if current_business_hour
    safe_closing_hour = current_business_hour.close_at - 30.minutes
    if current_time.strftime('%H:%M') >= '23:29'
      next_opening_hour = business_hours.where(week_day: DateTime.now.tomorrow.wday).order(:open_at).first
      if next_opening_hour
        if next_opening_hour.open_at.strftime('%H:%M') == '00:00'
          if next_opening_hour.close_at.strftime('%H:%M') >= '00:29'
            true
          elsif (next_opening_hour.close_at - 30.minutes).strftime('%H:%M') < current_time.strftime('%H:%M')
            false
          else
            true
          end
        else
          false
        end
      else
        false
      end
    elsif safe_closing_hour.strftime('%H:%M') < current_time.strftime('%H:%M')
      false
    else
      true
    end
  else
    false
  end
end


It's an unappealing spiderweb of if else statements, I do hope that the code makes someone what sense when you read it, then it's just the puzzle of understanding which true and false belongs to which.

I also feel like this is awfully complicated, and I wish that there was an easier way to do this.

Each store has many business hours, which has the week_day and an open_at and `close_

Solution

First I must disclose that I have no experience about programming in Ruby, I hope that the following is applicable.

In a comment I suggested Unix time stamps because they represent a continuous flow of time (integer number of seconds since 1.1.1970), and are therefore easy to use in time arithmetics and comparisons. Hopefully Ruby has support for Unix timestamps.

I also suggest that you store open hours in a db table, regardless of whether they extend over midnight, as a start time and an end time.

Define Unix30min = 30 x 60

Fetch UnixCurrentTime

Fetch a record from the db where (UnixOpenHoursOpen UnixCurrentTime)

The logic is then simplified to three if statements, without nesting. As you, I dislike deeply nested if, then elseif else end statements, they make my head hurt. If possible I split complex logic into simple statement, readability and maintainability improves significantly.

if nothing was returned (from the db), the shop is closed

if ((UnixCurrentTime > UnixOpenHoursOpen) and (UnixCurrentTime  (UnixOpenHoursClose - Unix30min)) and (UnixCurrentTime < UnixOpenHoursClose))
{
  exit(ShopIsClosing)
}


The above is pseudocode

Code Snippets

if nothing was returned (from the db), the shop is closed

if ((UnixCurrentTime > UnixOpenHoursOpen) and (UnixCurrentTime < (UnixOpenHoursClose - Unix30min)))
{
  exit(ShopIsOpen)
}

if ((UnixCurrentTime > (UnixOpenHoursClose - Unix30min)) and (UnixCurrentTime < UnixOpenHoursClose))
{
  exit(ShopIsClosing)
}

Context

StackExchange Code Review Q#126425, answer score: 2

Revisions (0)

No revisions yet.