Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior and infinite loops when using 24:00 #113

Closed
ahouston opened this issue Jun 6, 2019 · 4 comments
Closed

Unexpected behavior and infinite loops when using 24:00 #113

ahouston opened this issue Jun 6, 2019 · 4 comments

Comments

@ahouston
Copy link

ahouston commented Jun 6, 2019

I'm experiencing some strange problems with 2.3.0 when adding an hour range of ['00:00-24:00'] on a day.

Problem 1: When using nextOpen() the result will show the next week instead of the next day
Problem 2: When creating the object with an array instead of static statement, there is an infinite loop somewhere which causes a hang.

This seems to be quite similar to issue #44 - Bug with next open when opening times are set to 00:00, except that it affects the 24:00.

Here is some example code:

<?php
  
use Spatie\OpeningHours\OpeningHours;

require_once __DIR__.DIRECTORY_SEPARATOR.'vendor/autoload.php';


$dayHours = [];
foreach (array('monday','tuesday','wednesday','thursday','friday','saturday','sunday') as $day) {
        $dayHours[$day] = ['00:00-24:00'];
}

$scheduleHangs = OpeningHours::create($dayHours);

$scheduleMidnight = OpeningHours::create([
    'monday'    => ['00:00-24:00'],
    'tuesday'   => ['00:00-24:00'],
    'wednesday' => ['00:00-24:00'],
    'thursday'  => ['00:00-24:00'],
    'friday'    => ['00:00-24:00']
]
);

$scheduleAlmostMidnight = OpeningHours::create([
    'monday'    => ['00:00-23:59'],
    'tuesday'   => ['00:00-23:59'],
    'wednesday' => ['00:00-23:59'],
    'thursday'  => ['00:00-23:59'],
    'friday'    => ['00:00-23:59']
]
);

// Thursday 06 June 2019 19:02:00
var_dump($scheduleMidnight->nextOpen(new DateTime("2019-06-06 19:02:00")));
var_dump($scheduleHangs->nextOpen(new DateTime("2019-06-06 19:02:00")));
var_dump($scheduleAlmostMidnight->nextOpen(new DateTime("2019-06-06 19:02:00")));


?>

The result of this is an incorrect value on the first var_dump, and a hang on the second:

object(DateTime)#63 (3) {
  ["date"]=>
  string(26) "2019-06-10 00:00:00.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(19) "Africa/Johannesburg"
}
^C

Changing the "00:00" to "00:01" has no effect, but changing "24:00" to "23;59" resolves the issue with all three of the var_dump() statements returning the correct value of "2019-06-07 00:00:00".

object(DateTime)#63 (3) {
  ["date"]=>
  string(26) "2019-06-07 00:00:00.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(19) "Africa/Johannesburg"
}
object(DateTime)#60 (3) {
  ["date"]=>
  string(26) "2019-06-07 00:00:00.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(19) "Africa/Johannesburg"
}
object(DateTime)#63 (3) {
  ["date"]=>
  string(26) "2019-06-07 00:00:00.000000"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(19) "Africa/Johannesburg"
}

@kylekatarnls
Copy link
Collaborator

First of all the result "2019-06-10 00:00:00.000000" is correct, because "2019-06-06 19:02:00" is Thursday and so open, it does not close until Saturday 00:00 (since it's continuously open from Monday to Friday included), and re-open on Monday 00:00 (the next open "2019-06-10 00:00:00.000000"). Why would you expect an other result?

Then 23:59 is different from 24:00 because it means it's closed 1 minute each night.

I will fix the infinite loop bug. But unless you can explain why it's incorrect, the outputs will not change.

@kylekatarnls kylekatarnls added bug and removed bug labels Jun 6, 2019
@kylekatarnls
Copy link
Collaborator

In fact the infinite loop is not a bug neither but a rather a not supported case. As it's continuously open every day, we cannot find the next close and so the next open neither, and we iterate until infinity. As you could pass exceptions, for example '2035-05-12' => ['00:00-02:00'] it justifies we continue to search for the possible next open.

Maybe we could detect if there are exceptions in the future and else, stop iterating after a week, then return null or throwing an exception.

@kylekatarnls
Copy link
Collaborator

The patch 2.3.1 introduces a iteration limit (8 days if there is no exceptions, 366 days else) and this limit can be customized with ->setDayLimit()

I close this issue as for me there are 2 different and not related subjects here. I consider the infinite loop as fixed.

About the fact you expect the same result using "24:00" or "23;59" I can't be agree, opening-hours is minute-precise so 24:00 = 00:00 on next day, while 23:59 is 1 minute before.

If you still think, we should handle it differently or if you find inconsistencies in outputs, please open an other issue with what you expect and why, and maybe a real use-case for it because exemples of ever closed/ever open would not make too much sense according to the purpose of this library

@ahouston
Copy link
Author

ahouston commented Jun 7, 2019

Hi Kyle, thanks for the detailed feedback.

I understand your reasoning with the 24:00 vs 23:59 - I'll modify my code to work with that. I'm probably using this outside of your intended use-case as I'm using it to process alert schedules for an event system, so "always open" or 24x7x365 is one of the possible schedules that would probably not be usual for a business!

Thanks for the 2.3.1 patch, much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants