briannesbitt / Carbon

A simple PHP API extension for DateTime.
https://carbon.nesbot.com/
MIT License
16.58k stars 1.28k forks source link

"Inverted" CarbonPeriod with custom CarbonInterval #3070

Open eddi13 opened 2 months ago

eddi13 commented 2 months ago

Hello.

Carbon version: 3.8.0

PHP version: 8.2.23

I would like to get a period in "reverse" order (start date is greater than end date). In "normal" cases it works:

$startDate = Carbon::create('2024-09-12')->startOfDay();
$endDate = Carbon::create('2024-09-5')->startOfDay();

$twoDaysInterval = new CarbonInterval();
$twoDaysInterval->subDays(2);
$twoDaysPeriod = $twoDaysInterval->toPeriod($startDate, $endDate);//works: 2024-09-12, 2024-09-10, 2024-09-08, 2024-09-06
echo "twoDaysPeriod: {$twoDaysPeriod->count()}<br>";
foreach($twoDaysPeriod as $date) {
    echo $date->toDateString();
    echo "<br>";
}

But I need a customized CarbonInterval because I have to subtract the working days. The closure does not work properly because $negated is always passed as FALSE (is a bug?). Regardless of whether CarbonInterval is inverted or not:

$startDate = Carbon::create('2024-09-12')->startOfDay();
$endDate = Carbon::create('2024-09-5')->startOfDay();

$twoWeekdaysInterval = new CarbonInterval(function (CarbonInterface $date, bool $negated): DateTime {
    // $negated is true when a subtraction is requested, false when an addition is requested
    return $negated
        ? $date->subWeekdays(2)
        : $date->addWeekdays(2);
});
$twoWeekdaysInterval->subDays(1);//is inverted
$twoWeekdaysPeriod = $twoWeekdaysInterval->toPeriod($startDate, $endDate);//not works
//ErrorException: E_WARNING: DateTime::modify(): Failed to parse time string (10000-01-04 00:00:00.000000) at position 12 (0): Double time specification inc\vendor\Carbon\src\Carbon\Traits\Modifiers.php [442]
echo "twoWeekdaysPeriod (uses 'negated'): {$twoWeekdaysPeriod->count()}<br>";
foreach($twoWeekdaysPeriod as $date) {
    echo $date->toDateString();
    echo "<br>";
}

I tried to trick and always subtract in closure and this works:

$startDate = Carbon::create('2024-09-12')->startOfDay();
$endDate = Carbon::create('2024-09-5')->startOfDay();

$twoWeekdaysInterval = new CarbonInterval(function (CarbonInterface $date, bool $negated): DateTime {
   return $date->subWeekdays(2);
});
$twoWeekdaysInterval->subDays(1);
$twoWeekdaysPeriod = $twoWeekdaysInterval->toPeriod($startDate, $endDate);//works: 2024-09-12, 2024-09-10, 2024-09-06
echo "twoWeekdaysPeriod: {$twoWeekdaysPeriod->count()}<br>";
foreach($twoWeekdaysPeriod as $date) {
    echo $date->toDateString();
    echo "<br>";
}

But I have the feeling that the code is not clean - it's a trick. Maybe I'm thinking in the wrong direction. What would be the "right" way to create such a "reverse" CarbonPeriod?

Thanks in advance!

kylekatarnls commented 2 months ago

Hello, step interval are a bit special because they can be either positive or negative, in a non-predictible way, the order can even be variable:

$i = 0;
$period = CarbonPeriod::create('2024-09-11', 4, static function (CarbonInterface $date) use (&$i): CarbonInterface {
    return match(++$i) {
        1 => $date->addDay(),
        2 => $date->subDays(2),
        3 => $date->addDays(3),
        default => $date,
    };
});

Will result in:

2024-09-11
2024-09-12
2024-09-10
2024-09-13

As you correctly found out, currently the direction of the period is driven by the interval (assumed descending if inverted, else ascending).

The logic for that is here: https://github.com/briannesbitt/Carbon/blob/3.8.0/src/Carbon/CarbonPeriod.php#L2316

So instead of $twoWeekdaysInterval->subDays(1) to trigger this behavior you can rather use:

$twoWeekdaysInterval->invert($endDate < $startDate);

At the moment, that the cleanest way I can found.

You can also completely replace the filters of the period (which decide when to stop iterating), that is the more "correct" way but the verbosity of it is likely not worth it.

$descending = ($endDate < $startDate);
$twoWeekdaysPeriod = CarbonPeriod::create($startDate, $endDate, function (CarbonInterface $date) use ($descending): CarbonInterface {
    return $descending
        ? $date->subWeekdays(2)
        : $date->addWeekdays(2);
})->setFilters([
    [function (CarbonInterface $date) use ($endDate, $descending): bool|callable {
        return ($descending
            ? ($date > $endDate)
            : ($date < $endDate)
        ) ?: CarbonPeriod::END_ITERATION;
    }, null]
]);

// Here you'll get your 3 dates
echo "twoWeekdaysPeriod: {$twoWeekdaysPeriod->count()}\n";
foreach($twoWeekdaysPeriod as $date) {
    echo $date->toDateString();
    echo "\n";
}
eddi13 commented 2 months ago

Thank you.

I have chosen the following option. Works for both reverse and normal periods.

//reverse period
$startDate = Carbon::create('2024-09-12')->startOfDay();
$endDate = Carbon::create('2024-09-5')->startOfDay();
//normal period
//$startDate = Carbon::create('2024-09-5')->startOfDay();
//$endDate = Carbon::create('2024-09-12')->startOfDay();

$descending = ($endDate->lessThan($startDate));
$twoWeekdaysInterval = new CarbonInterval(function(CarbonInterface $date, bool $negated) use($descending): DateTime {
    return $descending
        ? $date->subWeekdays(2)
        : $date->addWeekdays(2);
});
$twoWeekdaysInterval->invert($descending);
$twoWeekdaysPeriod = $twoWeekdaysInterval->toPeriod($startDate, $endDate); //works: 2024-09-12, 2024-09-10, 2024-09-06

echo "twoWeekdaysPeriod: {$twoWeekdaysPeriod->count()}<br>";
foreach($twoWeekdaysPeriod as $date) {
    echo $date->toDateString();
    echo "<br>";
}

However, it would be good if the closure was also passed the appropriate $negated (if inverted, TRUE). Then there is no need for tricks with 'use($descending)'.