Rewrite implementation using a more OOP / modern approach, to replace…

… used legacy code
fastbolt · Apr 14, 2023 · 93a3a81 · 93a3a81
1 parent 953dcdb
commit 93a3a81
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 64 deletions.
diff --git a/src/Configuration.php b/src/Configuration.php
@@ -34,9 +34,12 @@ public function __construct(array $config = [])
         $this->excludeWeekDays = $config['excludeWeekDays'] ?? $this->excludeWeekDays;
     }
 
+    /**
+     * @return self
+     */
     public static function getDefaultConfiguration(): self
     {
-        return new self;
+        return new self();
     }
 
     /**

diff --git a/src/WorkingDayProvider.php b/src/WorkingDayProvider.php
@@ -2,7 +2,12 @@
 
 namespace Fastbolt\WorkingDayProvider;
 
+use DateInterval;
+use DatePeriod;
+use DateTime;
+use DateTimeImmutable;
 use DateTimeInterface;
+use Fastbolt\WorkingDayProvider\Holiday\Holiday;
 use Fastbolt\WorkingDayProvider\Holiday\HolidayProvider;
 
 class WorkingDayProvider
@@ -22,73 +27,87 @@ public function __construct(?HolidayProvider $holidayProvider = null, ?Configura
     }
 
     /**
-     * @param DateTimeInterface $periodStart
-     * @param DateTimeInterface $periodEnd
+     * @param DateTime $periodStart
+     * @param DateTime $periodEnd
      *
      * @return int
      */
-    public function getWorkingDaysForPeriod(DateTimeInterface $periodStart, DateTimeInterface $periodEnd): int
-    {
-        $holidays  = null === $this->holidayProvider
+    public function getWorkingDaysForPeriod(
+        DateTime $periodStart,
+        DateTime $periodEnd
+    ): int {
+        $holidays = null === $this->holidayProvider
             ? []
-            : $this->holidayProvider->getHolidaysForDateRange($periodStart, $periodEnd);
-        $endDate   = $periodEnd->getTimestamp();
-        $startDate = $periodStart->getTimestamp();
-
-        //The total number of days between the two dates. We compute the no. of seconds and divide it to 60*60*24
-        //We add one to inlude both dates in the interval.
-        $days = ($endDate - $startDate) / 86400 + 1;
-
-        $numFullWeeks     = floor($days / 7);
-        $numRemainingDays = fmod($days, 7);
-
-        //It will return 1 if it's Monday,.. ,7 for Sunday
-        $firstDayOfWeek = (int)date('N', $startDate);
-        $lastDayOfWeek  = (int)date('N', $endDate);
-
-        //---->The two can be equal in leap years when february has 29 days, the equal sign is added here
-        //In the first case the whole interval is within a week, in the second case the interval falls in two weeks.
-        if ($firstDayOfWeek <= $lastDayOfWeek) {
-            if ($firstDayOfWeek <= 6 && 6 <= $lastDayOfWeek) {
-                $numRemainingDays--;
-            }
-            if ($firstDayOfWeek <= 7 && 7 <= $lastDayOfWeek) {
-                $numRemainingDays--;
+            : $this->indexHolidays($this->holidayProvider->getHolidaysForDateRange($periodStart, $periodEnd));
+
+        // set time to 00:00:00
+        $startDate = DateTimeImmutable::createFromMutable($periodStart)
+                                      ->setTime(0, 0, 0);
+
+        // set time to next day 00:00:00, otherwise end date is not included
+        $endDate = DateTimeImmutable::createFromMutable($periodEnd)
+                                    ->modify('+1 day')
+                                    ->setTime(0, 0, 0);
+
+        $oneDayInterval = DateInterval::createFromDateString('1 day');
+        $oneDayPeriod   = new DatePeriod($startDate, $oneDayInterval, $endDate);
+        $numDays        = 0;
+
+        foreach ($oneDayPeriod as $iteratorDate) {
+            if ($this->isExcludedDate($iteratorDate)) {
+                continue;
             }
-        } elseif ($firstDayOfWeek === 7) {
-            // if the start date is a Sunday, then we definitely subtract 1 day
-            $numRemainingDays--;
 
-            if ($lastDayOfWeek === 6) {
-                // if the end date is a Saturday, then we subtract another day
-                $numRemainingDays--;
+            if ($this->isHoliday($iteratorDate, $holidays)) {
+                continue;
             }
-        } else {
-            // the start date was a Saturday (or earlier), and the end date was (Mon..Fri)
-            // so we skip an entire weekend and subtract 2 days
-            $numRemainingDays -= 2;
-        }
 
-        //The no. of business days is: (number of weeks between the two dates) * (5 working days) + the remainder
-//---->february in none leap years gave a remainder of 0 but still calculated weekends between first and last day, this is one way to fix it
-        $workingDays = $numFullWeeks * 5;
-        if ($numRemainingDays > 0) {
-            $workingDays += $numRemainingDays;
+            $numDays++;
         }
 
-        //We subtract the holidays
+        return $numDays;
+    }
+
+    /**
+     * Helper method to index holidays by date in format `Y-m-d`
+     *
+     * @param Holiday[] $holidays
+     *
+     * @return array<string, Holiday>
+     */
+    private function indexHolidays(array $holidays): array
+    {
+        $result = [];
         foreach ($holidays as $holiday) {
-            $timestamp = strtotime($holiday->getDate()->format('Y-m-d'));
-            //If the holiday doesn't fall in weekend
-            if (
-                $startDate <= $timestamp
-                && $timestamp <= $endDate
-                && !in_array((int)date('N', $timestamp), $this->configuration->getExcludeWeekDays(), true)
-            ) {
-                $workingDays--;
-            }
+            $result[$holiday->getDate()->format('Y-m-d')] = $holiday;
         }
 
-        return (int)$workingDays;
+        return $result;
+    }
+
+    /**
+     * @param DateTimeInterface $iteratorDate
+     *
+     * @return bool
+     */
+    private function isExcludedDate(DateTimeInterface $iteratorDate): bool
+    {
+        $dayOfWeek = (int)$iteratorDate->format('N');
+
+        // exclude week days from configuration object
+        return in_array($dayOfWeek, $this->configuration->getExcludeWeekDays(), true);
+    }
+
+    /**
+     * @param DateTimeInterface     $iteratorDate
+     * @param array<string,Holiday> $holidays Array of holidays indexed by date in format `Y-m-d`
+     *
+     * @return bool
+     */
+    private function isHoliday(DateTimeInterface $iteratorDate, array $holidays): bool
+    {
+        $dateFormatted = $iteratorDate->format('Y-m-d');
+
+        return array_key_exists($dateFormatted, $holidays);
     }
 }
diff --git a/tests/unit/WorkingDayProviderTest.php b/tests/unit/WorkingDayProviderTest.php
@@ -3,7 +3,6 @@
 namespace Fastbolt\WorkingDayProvider\Tests;
 
 use DateTime;
-use DateTimeInterface;
 use Fastbolt\TestHelpers\BaseTestCase;
 use Fastbolt\WorkingDayProvider\Configuration;
 use Fastbolt\WorkingDayProvider\Holiday\HolidayProvider;
@@ -30,11 +29,10 @@ class WorkingDayProviderTest extends BaseTestCase
      * @dataProvider getWorkingDaysForPeriodWithoutHoliday
      */
     public function testGetWorkingDaysForPeriodWithoutHoliday(
-        DateTimeInterface $startDate,
-        DateTimeInterface $endDate,
+        DateTime $startDate,
+        DateTime $endDate,
         int $expectedWorkingDayCount
-    )
-    {
+    ): void {
         $provider = new WorkingDayProvider();
         $result   = $provider->getWorkingDaysForPeriod($startDate, $endDate);
 
@@ -45,11 +43,11 @@ public function testGetWorkingDaysForPeriodWithoutHoliday(
      * @dataProvider getWorkingDaysForPeriodDataProvider
      */
     public function testGetWorkingDaysForPeriod(
-        DateTimeInterface $startDate,
-        DateTimeInterface $endDate,
+        DateTime $startDate,
+        DateTime $endDate,
         array $holidays,
         int $expectedWorkingDayCount
-    ) {
+    ): void {
         $this->configuration->method('getExcludeWeekDays')
                             ->willReturn([6, 7]);
         $this->holidayProvider->method('getHolidaysForDateRange')