Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Inverted" CarbonPeriod with custom CarbonInterval #3070

Open
eddi13 opened this issue Sep 11, 2024 · 2 comments
Open

"Inverted" CarbonPeriod with custom CarbonInterval #3070

eddi13 opened this issue Sep 11, 2024 · 2 comments

Comments

@eddi13
Copy link

eddi13 commented Sep 11, 2024

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
Copy link
Collaborator

kylekatarnls commented Sep 11, 2024

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
Copy link
Author

eddi13 commented Sep 12, 2024

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)'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants