aeon-php / calendar Goto Github PK

When TimePeriod overlaps/abuts other TimePeriod there should be a way to merge them.

Describe the feature
Add immutable TimePeriods::add method

Expected behavior
add method should be immutable, it should initialize new collection and merge current + new period into it

Additional context

Describe the bug

The following doesn't do anything, but should move the datetime to the next saturday of the internal timezone.

$nextSaturday = DateTime::fromString('2022-10-25 15:00:00')->modify('next Saturday');

echo $nextSaturday->format('Y-m-d');

should be 2022-10-27 but results in 2022-10-25.

Expected behavior

modify should be identical to the implementation of \DateTimeImmutable.

Additional context

A possible solution would be to internally just call self::fromDateTime($this->toDateTimeImmutable()->modify($modifier)) instead of a custom implementation.

But I don't know if there is a reason for not using a custom logic. Is there?

Describe the bug
In ff7b809 composer requirements are changed:

- "php": "^7.4.2 || ~8.0 || ~8.1.10"
+ "php": "~8.1.10 || ~8.2"

Now it's hard to update from PHP 8.1.6 to 8.2 since 1.0.7 is compatible with 8.1.6 only, and 1.0.8 is compatible with 8.1.10+ only.

In multi server environment, this makes upgrade harder.

Expected behavior
Is there a particular reason for this? Can we have a version that's compatible with both 8.1.6 and 8.2?

Describe the feature
Add TimePeriods::start() : ?DateTime and TimePeriods::end() : ?DateTime methods to TimePeriods

Expected behavior
start - should return start of first time periods (sorted).
end - should return end of last time periods (sorted).

Additional context
Add any other context about how you are going to use that feature and why do you think others will benefit from it as well.

Describe the feature
Two TimePeriods should be merged into one by creating new instance

Expected behavior

Additional context

Describe the bug

phpbench was commented out but must be brought back.

Expected behavior

phpbench is available again and triggered as part of the CI.

Additional context

Was removed in PR #297

Describe the feature

With \DateTimeImmutable it's possible to use the spaceship operator <=> for sorting. As there is no operator overloading in PHP, it would be great to have an object oriented alternative to it for all components to prevent having to use helper functions.

Expected behavior

Have the following for sorting:

$iterator->uasort(
    static fn (NewsEntry$a, NewsEntry $b) => $a->date->spaceshipOperator($b->date)
);

Additional context

The naming spaceshipOperator is the thing blocking me from working on a PR for it. I can't seem to find any examples for naming online. @norberttech Do you have an idea for the naming of it?

Describe the bug
There is a different naming scheme when it comes to mutations of DateTime and Day.

For Day the methods are named plus* and minus* (for example plusDays($days) or minusDays($days)).
For DateTime the methods are named add* and sub* (for example addDays($days) or subDays($days)).

Expected behavior

I would expect that the naming follows the same schema so that both are ether using plus/minus or add/sub.

Additional context

I'm not sure if that's a bug or whether there is a specific reason. I looked through the documentation but couldn't find an explanation why the naming schemes are different and how I would know when to use which naming.

Is there a specific reason or just due to growth of the library and nobody noticed it until now? 🙂

UPDATE: I've also looked into Time and it only has add/sub without any shortcut methods. So I guess Day is the outlier.

Using DateTime::endOfDay, DateTime::midnight, DateTime::noon causes resetting the TimeZone.

Example:

$dt = \Aeon\Calendar\Gregorian\DateTime::create(2020, 6, 20, 20, 0, 0, 0, 'Europe/Warsaw');
echo $dt;

In result we have: 2020-06-20T20:00:00+0200

But in the result of

$dt = \Aeon\Calendar\Gregorian\DateTime::create(2020, 6, 20, 20, 0, 0, 0, 'Europe/Warsaw')->endOfDay();
echo $dt;

we have 2020-06-20T23:59:59+0000 and now the TimeZone is UTC

In order to improve user experience classes, Day and Month should get a new static constructor, similar to one that DateTime has.

• Day::create(int $year, int $month, int $day) : self
• Month::create(int $year, int $month) : self

So currently DateTime class makes possible to iterate by any deterministic TimeUnit like seconds, days, hours.
However sometimes it's required to iterate by months which is tricky by itself because each month has different number of days.

To solve that problem and provide a way to iterate by months/years/day class Month, Year, Day can expose following methods:

• Month::iterateUntil(self $month) : Months
• Month::iterateSince(self $month) : Months
• Day::iterateUntil(self $day) : Days
• Day::iterateSince(self $day) : Days
• Year::iterateUntil(self $year) : Years
• Year::iterateSince(self $year) : Years

In order to make this possible we would need to rename currently existing Days and Months classes into YearMonths and MonthDays which should be fine since those are internal classes. Years does not exists yet so it would need to be added.

Please consider resetting the time to 0:0:0 in Day/Month/Year::toDateTimeImmutable so the following won't bite you:

$day = new Day(new Month(new Year(2020), 6), 11);

$date1 = $day->toDateTimeImmutable();
sleep(1);
$date2 = $day->toDateTimeImmutable();

var_dump($date1 == $date2); // false

Describe the bug

The resulting internals of a new DateTime object differ depending on the used method of construction. This leads to failing tests depending on how the objects are constructed.

This works:

public function test_equals_works() : void
{
    $this->assertEquals(
        DateTime::fromDateTime(new \DateTimeImmutable('2022-01-01 15:00:00')),
        DateTime::fromString('2022-01-01 15:00:00')
    );
}

This doesn't:

public function test_equals_works() : void
{
    $this->assertEquals(
        DateTime::fromDateTime(new \DateTimeImmutable('2022-01-01 15:00:00')),
        new DateTime(
            new Day(
                new Month(
                    new Year(2022),
                    1
                ),
                1
            ),
            new Time(15, 0, 0),
            TimeZone::UTC()
        )
    );
}

and fails with:

Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 Aeon\Calendar\Gregorian\DateTime Object (
-    'day' => null
-    'time' => null
-    'timeZone' => null
-    'dateTime' => DateTimeImmutable Object (...)
+    'day' => Aeon\Calendar\Gregorian\Day Object (...)
+    'time' => Aeon\Calendar\Gregorian\Time Object (...)
+    'timeZone' => Aeon\Calendar\Gregorian\TimeZone Object (...)
+    'dateTime' => null
 )

Expected behavior

This should work:

public function test_equals_works() : void
{
    $this->assertEquals(
        DateTime::fromDateTime(new \DateTimeImmutable('2022-01-01 15:00:00')),
        new DateTime(
            new Day(
                new Month(
                    new Year(2022),
                    1
                ),
                1
            ),
            new Time(15, 0, 0),
            TimeZone::UTC()
        )
    );
}

Additional context

This might also impact the $clean logic within the time units like month which have the same issues.

Making the state identical for all methods on construction would also remove the side effects. For example when I construct a Time object from a string like 15:00:00 the hour, minute, ... properties won't be set. When I then call the getter hour() on it, the internal state of an "immutable" object is updated and the values are suddenly available which will might then change the test result.

I don't know whether this internal behaviour was chosen because it was the faster initial setup or whether there might be a specific reason where this solution was the only one the solve a specific issue?

Can you think of a workaround for this?

Describe the bug
It seems that test suite is failing at PHP 8.1, most likely due to php/php-src#7752

This could be really useful helper since now the only way is to do something like:

$diff = $day1->midnight(TimeZone::UTC())
            ->distanceSince($day2>midnight(TimeZone::UTC()))
            ->inDays();

API:
Day::daysBetween(self $day) : int

Describe the bug
If found another difference in naming when it comes to isAfter vs isGreaterThen and isAfterOrEqual vs isGreaterThanEq.

I think the difference is even greater then the previous one as we have an additional Then and Eq instead of Equal.

Expected behavior

I would expect that the naming follows the same schema so that all are using the same schema.

I'd prefer isAfter and isAfterOrEqual and the same for isBefore.

Additional context

Is there a reason for the naming or is it just something that fell down before the initial release? If so, I would provide a pull request to handle it.

Rename one method name in WeekDay to keep comparison method names consistent across all classes in the library.