patternrubyrailsMinor
Check if a store is currently open
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.
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
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
endIt'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
Fetch
Fetch a record from the db where
The logic is then simplified to three
The above is pseudocode
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 60Fetch
UnixCurrentTimeFetch 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.