spatie / period

Complex period comparisons
https://spatie.be/open-source
MIT License
1.6k stars 72 forks source link

Why does diff sometimes return a gap? #76

Closed ameenross closed 3 years ago

ameenross commented 3 years ago

I interpreted the Period::diff function to be a "subtraction". So I can remove, say, a week from a month (whether or not they become 2 split periods). However, I noticed a special case in code that confused me and seems to do something else entirely: it returns a gap.

Right at the beginning of the function it checks whether the input was a single, non-overlapping period. In that case it returns the gap between them! But why is that? It's not consistent with the way diffSingle works (which is called in every other scenario). It seems to be an inconsistency.

In fact, diffSingle has a (conflicting) special case: it returns both of the periods. That makes no sense to me whatsoever. Here's some code:

// Period for entire month of January.
$january = new Period(
    new DateTimeImmutable('2021-01-01'),
    new DateTimeImmutable('2021-02-01'),
    Precision::SECOND,
    Boundaries::EXCLUDE_END
);

// Remove something (in this case nothing needs to be removed).
$firstPartOfJanuary = $january->diff(new Period(
    new DateTimeImmutable('2021-03-01'),
    new DateTimeImmutable('2021-04-01'),
    Precision::SECOND,
    Boundaries::EXCLUDE_END
))[0];

// Expected to be January. It turns out to be Februari!
$firstPartOfJanuary->getStart()->format('c'); // 2021-02-01T00:00:00+01:00
$firstPartOfJanuary->getEnd()->format('c');   // 2021-02-28T23:59:59+01:00

/** Doing the exact same thing with diffSingle. */

$partsOfJanuary = $january->diffSingle(new Period(
    new DateTimeImmutable('2021-03-01'),
    new DateTimeImmutable('2021-04-01'),
    Precision::SECOND,
    Boundaries::EXCLUDE_END
));

// Expected to be January, and it is. But it also includes March!
$partsOfJanuary[0]->getStart()->format('c'); // 2021-01-01T00:00:00+01:00
$partsOfJanuary[0]->getEnd()->format('c');   // 2021-02-01T00:00:00+01:00
$partsOfJanuary[1]->getStart()->format('c'); // 2021-03-01T00:00:00+01:00
$partsOfJanuary[1]->getEnd()->format('c');   // 2021-04-01T00:00:00+01:00

/** Doing the exact same thing, but remove a second, bogus, period as well. */

$partsOfJanuary = $january->diff(
    new Period(
        new DateTimeImmutable('2021-03-01'),
        new DateTimeImmutable('2021-04-01'),
        Precision::SECOND,
        Boundaries::EXCLUDE_END
    ),
    new Period(
        new DateTimeImmutable('9998-01-01'),
        new DateTimeImmutable('9999-01-01'),
        Precision::SECOND,
        Boundaries::EXCLUDE_END
    )
)[0];

// Expected to be January, and January *only*. And it is!
$partsOfJanuary[0]->getStart()->format('c'); // 2021-01-01T00:00:00+01:00
$partsOfJanuary[0]->getEnd()->format('c');   // 2021-02-01T00:00:00+01:00
count($partsOfJanuary);                      // 1
brendt commented 3 years ago

Let's further discuss this in https://github.com/spatie/period/pull/77