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

Next Open Hours feature #21

Merged
merged 2 commits into from
Jan 3, 2017

Conversation

Djuki
Copy link
Contributor

@Djuki Djuki commented Oct 18, 2016

Added feature to get next open hour for DateTime requested in #18

Example:

    $openingHours = OpeningHours::create([
        'monday' => ['09:00-11:00', '13:00-19:00'],
    ]);

    // 2016-09-26 13:00:00
    $nextTimeOpen = $openingHours->nextOpen(new DateTime('2016-09-26 12:00:00'));

Unit tests added for class OpeningHours but not for OpeningHoursForDay because it is also covered in the same tests.

There could be space for code improvement in method nextOpen, and I would like to improve it if you have some ideas.

New method nextOpen is documented and unit tested. Also, psr2 fixer is applied to the code changes.

@Djuki
Copy link
Contributor Author

Djuki commented Oct 19, 2016

@freekmurze I came up with more readable and understandable solution for OpeningHoursForDay::nextOpen method. I can push if you think it could be an acceptable option.

public function nextOpen(Time $time)
{
    foreach ($this->openingHours as $timeRange) {
        if ($nextOpen = $this->findNextOpenInWorkingHours($time, $timeRange)) {
            return $nextOpen;
        }

        if ($nextOpen = $this->findNextOpenInFreeTime($time, $timeRange)) {
            return $nextOpen;
        }
    }

    return false;
}

private function findNextOpenInWorkingHours(Time $time, TimeRange $timeRange)
{
    if ($timeRange->containsTime($time) && next($timeRange) !== $timeRange) {
        return next($timeRange);
    }
}

private function findNextOpenInFreeTime(Time $time, TimeRange $timeRange, TimeRange &$prevTimeRange = null)
{
    $timeOffRange = $prevTimeRange ?
        TimeRange::fromString($prevTimeRange->end().'-'.$timeRange->start()) :
        TimeRange::fromString('00:00-'.$timeRange->start())
    ;

    if ($timeOffRange->containsTime($time)) {
        return $timeRange->start();
    }

    $prevTimeRange = $timeRange;
}

@sebastiandedeyne sebastiandedeyne merged commit cfdce45 into spatie:master Jan 3, 2017
@sebastiandedeyne
Copy link
Member

Sorry for the very late merge.

All looks good, thanks for your contribution!

@sebastiandedeyne sebastiandedeyne mentioned this pull request Jan 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants