fix: return 0 length for empty periods instead of a positive count
Period::length() derived its count from diff()->days (day precision) and abs() (hour/minute/second precision), both of which discard sign information. For a period whose boundary exclusions push includedStart() past includedEnd() (e.g. a single-day period with Boundaries::EXCLUDE_ALL), the period is empty and iterating it yields zero dates, yet length() still reported a positive number, disagreeing with iterator_count(). Add an explicit emptiness check so length() returns 0 whenever includedStart() is after includedEnd(), matching the iterator's behaviour across all precisions.
This commit is contained in:
1 parent
a0a637fda7
commit
aeda5bd6bd
3 files changed
+33
-1
No files matched your search
@@ -66,6 +66,13 @@ trait PeriodGetters
|
||||
|
||||
public function length(): int
|
||||
{
|
||||
// When the boundary exclusions push the included start past the included
|
||||
// end, the period is empty: it contains no units and iterating over it
|
||||
// (see Period::getIterator()) yields nothing.
|
||||
if ($this->includedStart() > $this->includedEnd()) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
// Length of month and year are not fixed, so we can't predict the length without iterate
|
||||
// TODO: maybe we can use cal_days_in_month ?
|
||||
if ($this->precision->equals(Precision::MONTH(), Precision::YEAR())) {
|
||||
@@ -73,7 +80,9 @@ trait PeriodGetters
|
||||
}
|
||||
|
||||
if ($this->precision->equals(Precision::HOUR(), Precision::MINUTE(), Precision::SECOND())) {
|
||||
$length = abs($this->includedEnd()->getTimestamp() - $this->includedStart()->getTimestamp());
|
||||
// includedEnd is guaranteed to be >= includedStart here, so the
|
||||
// difference is already non-negative.
|
||||
$length = $this->includedEnd()->getTimestamp() - $this->includedStart()->getTimestamp();
|
||||
|
||||
if ($this->precision->equals(Precision::SECOND())) {
|
||||
return $length + 1;
|
||||
|
||||
@@ -88,3 +88,20 @@ dataset('expected_period_lengths', function () {
|
||||
yield [24, Period::make('2018-01-01 00:00:00', '2018-01-01 23:59:59', Precision::HOUR())];
|
||||
yield [24, Period::make('2018-01-01 00:00:00', '2018-01-02 00:00:00', Precision::HOUR(), Boundaries::EXCLUDE_END())];
|
||||
});
|
||||
|
||||
// Single-unit periods (start === end) whose boundary exclusions push the
|
||||
// included start past the included end, making them empty. The iterator
|
||||
// (Period::getIterator) yields nothing for these, so length() must return 0
|
||||
// too instead of a spurious positive count.
|
||||
dataset('empty_period_lengths', function () {
|
||||
yield 'day, exclude start' => [Period::make('2018-01-01', '2018-01-01', Precision::DAY(), Boundaries::EXCLUDE_START())];
|
||||
yield 'day, exclude end' => [Period::make('2018-01-01', '2018-01-01', Precision::DAY(), Boundaries::EXCLUDE_END())];
|
||||
yield 'day, exclude all' => [Period::make('2018-01-01', '2018-01-01', Precision::DAY(), Boundaries::EXCLUDE_ALL())];
|
||||
|
||||
yield 'hour, exclude all' => [Period::make('2018-01-01 10:00:00', '2018-01-01 10:00:00', Precision::HOUR(), Boundaries::EXCLUDE_ALL())];
|
||||
yield 'minute, exclude all' => [Period::make('2018-01-01 10:00:00', '2018-01-01 10:00:00', Precision::MINUTE(), Boundaries::EXCLUDE_ALL())];
|
||||
yield 'second, exclude all' => [Period::make('2018-01-01 10:00:00', '2018-01-01 10:00:00', Precision::SECOND(), Boundaries::EXCLUDE_ALL())];
|
||||
|
||||
yield 'month, exclude all' => [Period::make('2018-01-01', '2018-01-01', Precision::MONTH(), Boundaries::EXCLUDE_ALL())];
|
||||
yield 'year, exclude all' => [Period::make('2018-01-01', '2018-01-01', Precision::YEAR(), Boundaries::EXCLUDE_ALL())];
|
||||
});
|
||||
@@ -11,6 +11,12 @@ it('can determine the period length', function () {
|
||||
expect($period->length())->toEqual(15);
|
||||
});
|
||||
|
||||
it('returns a length of 0 for an empty period, consistent with its iterator', function (Period $period) {
|
||||
expect($period->includedStart())->toBeGreaterThan($period->includedEnd());
|
||||
expect($period->length())->toEqual(0);
|
||||
expect($period->length())->toEqual(iterator_count($period));
|
||||
})->with('empty_period_lengths');
|
||||
|
||||
it('can renew a period', function () {
|
||||
$period = Period::make('2018-01-01', '2018-01-15');
|
||||
|
||||
|
||||
Reference in new issue
Block a user