azuyalabs / yasumi

The easy PHP Library for calculating holidays
https://www.yasumi.dev
Other
1.04k stars 152 forks source link

List Work days between two dates #217

Closed flavioheleno closed 3 years ago

flavioheleno commented 4 years ago

Hi, is there an embedded way to list the work days between two given dates?

Something like: AbstractProvider::between, but for work days rather than holidays.

If not, I'm happy to provide a PR if this is an appropriate feature for the lib.

stelgenhof commented 4 years ago

Hi @flavioheleno Thank you for your question.

There is no such method within the API. I would recommend to create your own custom filter. You can use the BetweenFilter as an example: https://github.com/azuyalabs/yasumi/blob/develop/src/Yasumi/Filters/BetweenFilter.php

Only need to add a condition for checking if the holiday is a working day or not and you are good to go!

Cheers! Sacha

flavioheleno commented 4 years ago

Hi @stelgenhof Thank you for such a quick reply!

I've done something similar to that but not using a filter, your approach is def more elegant!

Would that filter be a nice thing to contribute back to the library?

stelgenhof commented 4 years ago

@flavioheleno Yes definitely! If you like to submit a PR that would be great.

Cheers! Sacha

flavioheleno commented 4 years ago

Hey @stelgenhof, I was checking the internals of the filter classes (specially the BetweenFilter class) and I could not find a way around to do Work Days with that approach.

The problem is that the iterator being filtered is an iterator of holiday dates and not the dates in the interval, check below.

    /**
     * Get an iterator for the holidays.
     *
     * @return ArrayIterator iterator for the holidays of this calendar
     */
    public function getIterator(): ArrayIterator
    {
        return new ArrayIterator($this->getHolidays());
    }

Would it be fine if I did something straight in the AbstractProvider using AbstractProvider::isWorkingDay?

That's pretty much how I did it in my solution (also below).

  public function workDays(DateTimeInterface $startDate, DateTimeInterface $endDate) : array {
    if ($startDate > $endDate) {
      throw new InvalidArgumentException('Start date must be a date before the end date.');
    }

    // Setup start date, if its an instance of DateTime, clone to prevent modification to original
    $date = $startDate instanceof DateTime ? clone $startDate : $startDate;

    // 1 day interval that will be used as increment
    $interval = new DateInterval('P1D');

    $workDayList = [];
    while ($date <= $endDate) {
      if ($this->isWorkingDay($date)) {
        $workDayList[] = clone $date;
      }

      $date = $date->add($interval);
    };

    return $workDayList;
  }
stelgenhof commented 4 years ago

Hi @flavioheleno Yes, I now realize I slightly misunderstood your request and also the isWorkingDay method is not a behavior of the Holiday class :(

If you could make a PR for your suggestion that would be great. Keep in mind that I am working on a different way how a working day is defined as the current logic is not appropriate. So your PR may come in a bit later or needs to be adjusted.

Cheers! Sacha

github-actions[bot] commented 4 years ago

This issue has been open 60 days with no activity. Please remove the stale label or comment, or this will be closed in 10 days.

github-actions[bot] commented 4 years ago

This issue has been open 60 days with no activity. Please remove the stale label or comment, or this will be closed in 10 days.

github-actions[bot] commented 3 years ago

This issue has been open 60 days with no activity. Please remove the stale label or comment, or this will be closed in 10 days.