我不得不计算重叠日期范围的总天数(在我的例子中来自MySQL ),不包括周末,下面是我所做的工作的一个版本。
在代码质量和可读性方面,我正在寻找一个更干净的解决方案,最理想的是原生Carbon解决方案。
有什么想法,如何改善这一点,或可能是一种完全不同的方法,我正在努力实现的?如果不是这样的话,我就把它留在这里,因为到目前为止,它似乎运转得很好。
样本输入:
//date ranges (end date can be omitted)
[
['2018-10-25', '2018-11-10'],
['2018-10-25', '2018-10-30'],
['2018-10-29', '2018-10-30'],
['2018-10-29', '2018-11-07'],
['2018-10-31', '2018-11-03'],
['2018-10-31', '2018-11-10']
]功能部分:
/**
* @param array $intervals
*
* @param bool $excludeWeekendDays
*
* @return int
*/
public static function calculateDaysByIntervals(array $intervals, $excludeWeekendDays = false) {
//placeholder to store dates
$days = [];
//now
$now = Carbon::now()->toDateTimeString();
//loop over provided date ranges
foreach ($intervals as $dateRange) {
//skip interval if start date is missing
if(!isset($dateRange[0])) {
continue;
}
try {
$startDate = Carbon::parse($dateRange[0])->toDateString();
$endDate = Carbon::parse($dateRange[1] ?? $now)->toDateString();
//get date periods
$period = CarbonPeriod::between($startDate, $endDate);
//only weekdays
if($excludeWeekendDays) {
$period->addFilter(function (Carbon $date) {
return $date->isWeekday();
});
}
foreach ($period as $date) {
//get date
$day = $date->format('Y-m-d');
//store day as array key
$days[$day] = 1;
}
//skip interval on exception
} catch(Throwable $x) {
continue;
}
}
return count($days);
}单元测试部分
/**
* @dataProvider additionProvider
*
* @param $dateRanges
* @param $excludeWeekends
* @param $expectedResult
*/
function testCalculateDaysByIntervals($dateRanges, $excludeWeekends, $expectedResult)
{
$actualResult = $this->DateTimeHelper->calculateDaysByIntervals($dateRanges, $excludeWeekends);
$this->assertEquals($expectedResult, $actualResult);
}
/**
* @return array
*/
public function additionProvider()
{
//put different sets of data
return [
[
//date ranges
[],
false, //exclude weekends
0 //result in days without weekends
],
[
//date ranges
[
['2018-10-6', '2018-10-7'] //Weekend
],
true, //exclude weekends
0 //result in days without weekends
],
[
//date ranges
[
['2018-10-6', '2018-10-7'] //Weekend
],
false, //exclude weekends
2 //result in days with weekends
],
[
//date ranges
[
['2018-10-1', '2018-10-1']
],
true, //exclude weekends
1 //result in days without weekends
],
[
//date ranges
[
['2018-10-1', '2018-10-10']
],
true, //exclude weekends
8 //result in days without weekends
],
[
//date ranges
[
['2018-10-25', '2018-10-30'],
['2018-10-29', '2018-10-30'],
['2018-10-29', '2018-11-07'],
['2018-10-31', '2018-11-03'],
['2018-10-31', '2018-11-10'],
],
true, //exclude weekends
12 //result in days without weekends
],
[
//date ranges
[
['2018-10-25', '2018-10-30'],
['2018-10-29', '2018-10-30'],
['2018-10-29', '2018-11-07'],
['2018-10-31', '2018-11-03'],
['2018-10-31', '2018-11-10'],
],
false, //exclude weekends
17 //result in days with weekends
],
[
//date ranges - invalid
[
['2018-10-25', '2017-10-05'], //start > end
['2018-10-25', 'Fake'], //can't parse
[null, 'Fake'], //will be skipped
],
false, //exclude weekends
0 //result in days with weekends
],
[
//date ranges
[
//'' will parse to NOW and null will be replaced with NOW
['', null]
],
false, //exclude weekends
1 //result in days with weekends
]
];
}发布于 2018-11-08 14:59:34
升级到php7以便使用严格的类型。(我认为类型安全是一种维护上的提升。)
我建议减少每种方法的行数。经验法则:一个方法不应该超过15行,因为任何更长的事情都很难解释。尝试找出可能是他们自己的方法的块。将来,您可能会将它们单独用作不同用例的助手。
另外,避免如果-否则嵌套。在大多数情况下,您可以通过以下操作保持嵌套级别不变:
if (conditon) {
return "foo" // or throw new Exception("Some Error!");
}
// continue will normal flow避开旗子。Martin有一篇不错的文章为什么要挂国旗?。
反对挂旗的要旨是:
我们可以通过具有特定名称的函数或多态性来以不同的方式解决这个问题。在我的示例重构中,我使用了一个不同的命名函数,即创建带周末和不带周末的时间段。
当您在数据提供程序中的单元测试设置中重新声明标志的用途时,您自己就意识到了这种糟糕的可读性。
为什么要设这么大的试捕块?为什么要抓住所有的投手?如果有什么东西坏了,就让它破了。如果某些异常应该使您的代码仍然是可执行的,则只捕获它们。但只试着抓住你方法的相关部分。不是整个。
你有很多评论:
//placeholder to store dates
//get date periods删除那些。他们是多余的。评论不应该描述如何和什么,而应该只描述原因。
例如,您可以:
//store day as array key
$days[$day] = 1;但你为什么不直接:
$day[] = $day;事实证明,后者生成重复,而您只对共享天数的计数感兴趣,而不是对总数感兴趣。这个推理是不沟通的,我必须理解,通过玩弄您的代码库。
把你的话说得更清楚。与其使用包含1个值的have,我更愿意这样做:
return \count(\array_unique($days));在最后。你不需要哈希图。(这是个挑剔的问题)
避免代码重复。在某些地方,即使差别可能是流线型的,你还是会在if- could块中重新做同样的事情。
以下是我的重构:
use Carbon\Carbon;
use Carbon\CarbonPeriod;
/**
* DateTimeHelper
**/
class DateTimeHelper
{
/**
* @param array $intervals
* @param bool $excludeWeekendDays
* @return int
*/
public static function calculateDaysByIntervals(array $intervals, bool $excludeWeekendDays = false): int
{
$days = [];
foreach ($intervals as $dateRange) {
list($startDate, $endDate) = static::getDates($dateRange);
$period = $excludeWeekendDays ? static::createPeriodWithoutWeekendDays($startDate, $endDate) : static::createPeriod($startDate, $endDate);
foreach ($period as $date) {
$day = $date->format('Y-m-d');
$days[] = $day;
}
}
return \count(\array_unique($days));
}
/**
* @param array $dateRange
* @param bool $removeTime
* @return array string[]
*/
private static function getDates(array $dateRange, $removeTime = true): array
{
$start = Carbon::parse($dateRange[0]);
$end = Carbon::parse($dateRange[1]) ?? Carbon::now();
if ($removeTime) {
return [$start->toDateString(), $end->toDateString()];
}
return [$start->toDateTimeString(), $end->toDateTimeString()];
}
/**
* @param $startDate
* @param $endDate
* @return CarbonPeriod
*/
protected static function createPeriod(string $startDate, string $endDate): CarbonPeriod
{
return CarbonPeriod::between($startDate, $endDate);
}
/**
* @param $startDate
* @param $endDate
* @return CarbonPeriod
*/
protected static function createPeriodWithoutWeekendDays(string $startDate, string $endDate): CarbonPeriod
{
return static::createPeriod($startDate, $endDate)->addFilter(function (Carbon $date) {
return $date->isWeekday();
});
}
}https://codereview.stackexchange.com/questions/207206
复制相似问题