3
\$\begingroup\$

I am working on a project that involves tracking the operating hours of businesses across different timezones. Some of these businesses operate past midnight, and to accommodate this, operating hours that extend past midnight are stored as 25:00 for 1 AM, 26:00 for 2 AM, etc., in a MySQL database.

I've written a PHP function check_operating_status that checks if a business, identified by $account_id, is currently open. This function takes into account the business's timezone and handles operating hours that span past midnight. I am particularly focused on ensuring this function:

  1. Correctly handles the timezone conversion, including DST cases
  2. Properly normalizes operating hours extending past midnight.
  3. Efficiently determines if the current time falls within the operating hours.
function check_operating_status($account_id, $check_time = 'now') {
    // Get the current date and time in the account's timezone
    $stmt_timezone = $pdo->prepare("SELECT timezone FROM accounts WHERE id = ?");
    $stmt_timezone->execute([$account_id]);
    $timezone = $stmt_timezone->fetchColumn();

    if ($timezone) {
        $date = new DateTime($check_time, new DateTimeZone($timezone));
        $weekday = $date->format('N');
        $current_time = $date->format('H:i:s');

        $previous_day = clone $date;
        $previous_day->modify('-1 day');
        $previous_weekday = $previous_day->format('N');

        // Check operating hours for today
        $stmt_hours = $pdo->prepare("SELECT open, close, weekday FROM operating_hours WHERE account_id = ? AND (weekday = ? OR weekday = ?)");
        $stmt_hours->execute([$account_id, $weekday, $previous_weekday]);
        
        $operating_hours = []; // Initialize an array to store the operating hours

        while ($row = $stmt_hours->fetch(PDO::FETCH_ASSOC)) {
            $open_time = $row['open'];
            $close_time = $row['close'];
            $day_of_week = $row['weekday'];
            // Check if the hours are for the previous day and need to be adjusted
            if ($day_of_week == $previous_weekday) {
                // Normalize hours that go past midnight
                if ($close_time > "24:00:00") {
                    list($hours, $minutes, $seconds) = explode(':', $close_time);
                    $hours = $hours % 24;
                    $normalized_close_time = sprintf('%02d:%02d:%02d', $hours, $minutes, $seconds);
                    // Add as a new entry for the current day
                    $operating_hours[] = [
                        'open' => '00:00:00',
                        'close' => $normalized_close_time
                    ];
                }
            } elseif ($day_of_week == $weekday) {
                // Store the original times for the current day if they do not need adjustment
                $operating_hours[] = [
                    'open' => $open_time,
                    'close' => $close_time > "24:00:00" ? "24:00:00" : $close_time // Limit the closing time to 24hrs
                ];
            }
        }
        // Check if current time falls within operating hours
        foreach ($operating_hours as $hours) {
            if ($current_time >= $hours['open'] && $current_time < $hours['close']) {
                return true; // Current time is within operating hours
            }
        }
    }
    return false;
}

I'm looking for feedback on:

  • Code Efficiency: Are there redundancies or inefficiencies in my approach that could be streamlined?
  • Timezone Handling: Is my method of adjusting and comparing times across timezones robust enough to handle edge cases?
  • Database Interaction: Could the querying and handling of operating hours be optimized further to reduce database load or improve performance?
  • General Best Practices: Are there PHP or general programming best practices I might be overlooking or could better incorporate?

Any insights, optimizations, or critiques would be greatly appreciated as I aim to refine this function and ensure its reliability and efficiency.

\$\endgroup\$
0

1 Answer 1

4
\$\begingroup\$

data representation

hours that extend past midnight are stored as 25:00 for 1 AM ...

With all due respect, that's just silly. It's going to increase your maintenance costs and make it harder to attract maintainers to your project. A bad data representation is more serious than a bad implementation, and can do more long term harm. I am especially worried that this representation will lead developers and maintainers astray, so they don't correctly think about DST issues. Timezones are subtle, and should be handled via appropriate library code. Performing app-specific arithmetic operations on timestamps is usually the wrong approach.

There are two kinds of establishments:

    1. Those that are open 24x7 without ever closing, and
    1. Those which are sometimes closed.

The OP is focused on that second kind. Among them we find

  • 2.1. Those with an irregular schedule.
  • 2.2. Those with a recurring daily schedule based on weekday / holiday status.
  • 2.2.1. Those which sometimes are open >= 24 hours, e.g. 05:00 Saturday .. 06:00 Sunday.
  • 2.2.2. Those which always close within 24 hours of opening, e.g. 05:00 Saturday .. 04:00 Sunday.

Many establishments are open 09:00 to 17:00 (9 < 17), and we never compute mod 24 congruent to zero while they're open. For type (2.2.2.) we can detect the zero crossing by noting 5 > 4. I see little advantage from storing 24 + 4 == 28 as a closing time.

If any of the remaining types are of interest for your use case, there's little choice, you pretty much need to store date + timestamps for the instants of opening and closing. This is quite flexible and accommodates an establishment that opens at 17:00 Friday and closes 09:00 Monday.

holidays

The OP code pays attention to weekday, without considering whether a business might change its schedule for Easter or Christmas. At some point you likely will want a table of upcoming holidays.

post-condition

function check_operating_status( ... ) {

This could be a more informative identifier, such as is_open() or is_operating(). Failing that, at least tell us what promises this function makes. For example, it turns out that instead of evaluating it for side effects, it is actually a predicate that always returns a boolean. And then we'll want to know how to interpret what true means. Some /** documentation comments */ would help with that.

extract helper

This is a useful comment, and I thank you.

    // Get the current date and time in the account's timezone

It would make an even better helper function name.

meaningful identifiers

        $date = new DateTime($check_time, new DateTimeZone($timezone));

Prefer to call that a $timestamp, since it contains more than just Y-m-d. It properly models an instant in time, an instant which happened simultaneously everywhere in the world, albeit with different spellings for folks in different cities.

misleading comment

        // Check operating hours for today

This is true in the 'now' default case, but in general that's not what we're checking here. Comments that lie affect how we reason about code, and can do much harm.

query params

SELECT open, close, weekday
FROM operating_hours
WHERE ... AND (weekday = ? OR weekday = ?)

I don't understand what that's computing. I hope you insist on each account_id having 7 rows in that table, and that weekday corresponds to the opening time. We should be filtering on just a single weekday.

The quantities you're computing, $weekday and $previous_weekday, are not the quantities of interest, because they relate to the input $check_time. What we care about is the {open, close} timestamps of the business, something your RDBMS does not appear to currently model.

I recommend you keep operating_hours as a "rule" table with 7 rows per business, and leverage it to populate a bigger table with actual {open, close} timestamps. Then your DB query becomes trivial, perhaps using a BETWEEN operator. (Well, that's a closed interval; what we actually want is a half-open interval.) Storing timestamps lets you avoid those // need to be adjusted details.

UTC

Ideally you would use the "local time" rules to generate and store UTC timestamps. Then all computations and comparisons on the inside of your system become extremely simple and uniform.

The DST details get handled automatically when you convert to UTC on the way in, and convert from UTC on the way out. I worked on a large production system which had timezone bugfixes reported / repaired every six months, whenever U.S. DST changed, due to some new hire forgetting a detail. Then we changed all internal storage to UTC, and maintenance costs went way down, it became easy to unit test and to add new use cases. (Hint: Phoenix wristwatches sometimes match Denver, and sometimes match Los Angeles. Phoenix is in a timezone, but it is in neither of those two.)

I will note in passing that timestamp columns are good targets for CREATE INDEX ...

unit tests

This submission contains no automated test suite. Believe me, you need one. If nothing else, legislators will decide to switch the day DST begins at some point, and you'll want to verify Red bar with unmodified TZ database, and then Green bar once it has been updated.


This codebase does not appear to achieve its design goals.

I would not be willing to delegate or accept maintenance tasks on it in its current state.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.