azuyalabs / yasumi

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

Timezone is overridden in initialize method #69

Closed patrickreck closed 4 years ago

patrickreck commented 6 years ago

The timezone is overridden in the initialize method in most providers, making it very annoying to extend. Is this intentional?

/**
 * Initialize holidays for Denmark.
 */
public function initialize()
{
    $this->timezone = 'Europe/Copenhagen';

I propose setting it as a class property in each provider outside the scope of initialize.

stelgenhof commented 6 years ago

Setting the timezone in the initialize method is intentional in the sense that I thought it would be a good place to put. I think a class property would be equally good in my opinion.

Would you mind sharing in what situation you like to extend the timezone? (Just curious as I can't think of such a scenario :) ). Happy to change this if it better fits your needs.

patrickreck commented 6 years ago

I can only see it being an unnecessary limitation.

In Denmark we have some holidays that only some companies use, this being First of May and our Constitution Day. These are not present in the Denmark provider, and I guess that's ok because it's not really everyone who uses them.

I would guess that I am not the only one who for some reason would like to extend one of the existing providers :-)

stelgenhof commented 6 years ago

Thank you for the feedback. Understand that you'd like to extend the HolidayProvider class to add less common holidays. If these additional holidays are in the same timezone, how does the timezone initialization being a limitation?

I agree that perhaps a class constant or property maybe better; just trying to understand your issue :)

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.