Substitute holidays affect count

[alexdembo]

Is it right that when I do count on period where substitute holiday and the original holiday fall in, I get 2 holidays in count method?

[stelgenhof]

Good question! I haven't really tried it myself but probably yes. Since internally the list of holidays in a provider class will maintain the original holiday and the substitute holiday, both will be accounted for.

[alexdembo]

Thank you for such fast response. It seems that currently there is no way to filter out the substituted holidays. Do you see it as a field to contribution? If so, is it an additional property, and a filter?

[stelgenhof]

There is not a particular field or property for that. Only the internal name of the substituted has a prefix "substituteHoliday:". That may be used for filtering.

[stelgenhof]

@alexdembo Just wanted to check with you if my last suggestion works for you.

[alxdembo]

Unfortunately, no. Here is why: If you are trying to count holidays in a particular time span, you can not tell if the time span includes a holiday, that is substituted.

I will definitely come back to this thread with a solution if there wouldn't be one by that time.

Thanks for the support and great project, I really appreciate it.

[stelgenhof]

Thanks for the update! I understand your situation better now. For the moment I think the best solution is to correct the code to filter out the duplicate count. Don't think it will be that hard. :)

[stelgenhof]

@alxdembo I've made enhancements so that substitute holidays are only counted once. These changes are not yet part of a release, but you can give it a try using the master branch.

[alxdembo]

Thank you so much, Sacha! That's perfect. I'll test that asap.

[daffoxdev]

As I see only some countries using "substituteHoliday:" prefix. So may would be better to add some property to Holiday object and reference to substituted object? It will be very helpful when you filter.

I have made on application side the wrapper that can calculate holidays between dates. For example we want all holidays between 18/02/2018 - 21/07/2020 - as you can see here is two years. Abstract logic is: I have using your library to call holidays for each year and then filter it by selected dates. It is okey, it works pretty good. But, I have one problem with substitutions. You can see for USA, that date 04/07/2020 is substituted with 03/07/2020. But if I will filter holidays from 04/07/2020 to some date there will not the substitute date for 04. I will not even noticed that this day was substituted. There is a marker as prefix for day that is substitution but nothing for substituted date.

I think that prefix is bad idea to mark some type of entity. Need to create some public interface of Holiday, that allows to set flag that Holiday is substituted and set the reference to substitution Holiday Object.

Maybe something like that:

<?php

$holiday = new Holiday('shortName', ['en_US' => 'Full Name'], ...); 
$substituteHoliday = clone $holiday;
$substituteHoliday->setDate(2020, 7, 3);

$holiday->substituteHoliday($substituteHoliday);
$substituteHoliday->substitutedHoliday($holiday);

if ($holiday->getSubstitute()) {
...
}

[stelgenhof]

Thanks @daffoxdev! I agree that using the 'substitute' prefix isn't the best idea :) The idea of a property/indicator that the holiday is substituted is definitely a better one.

Also I like your idea of making a reference to the substituted holiday. Need to check how much that will impact the library.

Cheers! Sacha

[stelgenhof]

@alexdembo, @daffoxdev Recently a new class named SubstituteHoliday has been created that handles substitute holidays in a much better fashion.

The AbstractProvider::count method will not double count when a holiday is being substituted. You can of course still use the standard PHP count function, but that will count both the original holiday as well as the substitution. So depending on your needs, you can choose which function to use :)

All the filters have been adjusted accordingly as well with an AbstractFilter handling the number of holidays correctly (i.e. no double count).

Let me know if this solves your issue. (Please close this issue if you it has indeed solved it for you).

Cheers! Sacha

[stelgenhof]

Closing this issue as I haven't had any feedback yet and believe release v2.2.0 covers this.