brick / date-time

Date and time library for PHP
MIT License
337 stars 30 forks source link

Implement TemporalAdjusters or similar mechanism #20

Open kagmole opened 4 years ago

kagmole commented 4 years ago

Stop me if I'm wrong, but there is no simple, built-in way to do operations like the following in Carbon:

$nowDate = CarbonImmutable::now();

$previousWeekDate = $nowDate->subWeek();
$startOfPreviousWeekDate = $previousWeekDate ->startOfWeek();

The ThreeTen way is the use of TemporalAdjuster:

LocalDate nowDate = LocalDate.now();

// Custom temporal adjuster
TemporalAdjuster previousWeekAdjuster = t -> t.minus(Period.ofDays(7));

// Static, utility temporal adjusters
TemporalAdjuster firstMondayAdjuster = TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY);

LocalDate previousWeekDate = nowDate.with(previousWeekAdjuster);
LocalDate startOfPreviousWeekDate = previousWeekDate.with(firstMondayAdjuster);

If it is OK, I would like to investigate and if possible (most likely) implement the ThreeTen way.

BenMorel commented 4 years ago

Hi, you can subtract a week with ->minusWeeks(1), however indeed, there is no way to get the previous Monday.

I'll happily accept a PR for this; however, you don't necessarily have to do it the Java way, you may just add a previousOrSame(int $dayOfWeek) method to LocalDate!

kagmole commented 4 years ago

I try to stick to the ThreeTen API since this is the base of the project. This is to keep the API consistent with other ThreeTen backports like JS Joda as well.

But if you feel this is going to be too cumbersome, let me know.

BenMorel commented 4 years ago

TBH this library has already deviated a bit from ThreeTen (for better of for worse), and I'm not sure whether I want to continue walking in their footsteps.

For now, I'd accept a PR with only the method in question. If there's a plan to stick to the Threeten API, we can still do it later, there will be a BC break anyway.

I do like the foundation layed by Threeten, however I feel like their API is a bit bloated, and this particular use case "give me the previous monday" is a good example IMO.

What else can TemporalAdjusters do, exactly?

kagmole commented 4 years ago

A TemporalAdjuster is just a lambda (SAM interface). It takes one argument, the object being modified, and returns the modified version of the same type.

Since this is a lambda, you may create a more complex TemporalAdjuster than just "give me monday":

TemporalAdjuster previousMondayAdjuster = (Temporal baseT) -> {
    Temporal previousWeekT = baseT.minus(Period.ofDays(7));
    Temporal previousMondayT = previousWeekT.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY));

    return previousMondayT;
};

The use site then ends up like this:

LocalDate nowDate = LocalDate.now();

LocalDate previousMondayDate = nowDate.with(previousMondayAdjuster);

It is basically a way to allow developers to create custom manipulation "methods". Rather than fixing a set of methods like:

You only give one with(..) method that accepts a lambda.

But you got a point since simple operations like "substract one week" do have utility methods (minusWeeks(..)) in addition to the with(..) method. We could add simpler utility method like the one you mentioned or similar, like LocalDate.withDayOfWeek(DayOfWeek.MONDAY).

Note: TemporalAdjuster works on Temporal, which is the base interface for all temporal classes in the original ThreeTen. That is another "pro", it works on any temporal class.

Anyway, I will dig into the subject a little more. I will post additional feedbacks as to "why TemporalAdjuster exists" if I get some. :)

BenMorel commented 4 years ago

This is interesting, although I'm still not sure if I see a value for now. First of all, I can see an issue with returning a generic Temporal: we lose the type-hint in PHP, forcing to type-hint returned values as e.g. /** @var LocalDate $var */ to keep static analysis capabilities, which I don't like.

And, what's the point of creating your own Adjuster:

class FirstMondayAdjuster implements TemporalAdjuster {
    ...
}

$adjuster = new FirstMondayAdjuster();
$adjustedDate = $localDate->with($adjuster);

...when you can just create a class that does adjustments directly?

class Adjuster {
    public function firstMonday(LocalDate $localDate) : LocalDate {
        ...
    }
}

$adjuster = new Adjuster();
$adjustedDate = $adjuster->firstMonday($localDate);

We could add simpler utility method like the one you mentioned or similar, like LocalDate.withDayOfWeek(DayOfWeek.MONDAY).

I'd be happy with previousOrSame($dayOfWeek) and nextOrSame($dayOfWeek), to at least borrow some naming conventions from ThreeTen.

jurchiks commented 3 years ago

I believe he wants the equivalent of https://www.php.net/manual/en/datetime.modify.php except not a string modifier but more advanced. PHP does not support callback type hinting though (yet), so that would be a small problem... Since this project has a Duration class, you could accept that as an argument instead, the only problem would be the direction (+/-). It is a shame that this project's Interval class is not the same as PHP's built-in https://www.php.net/manual/en/class.dateinterval.php, because if it was, you could have used that instead.