Added data structure for exposing country descriptions

svendiedrichsen / jollyday

Jollyday - A holiday API

Other

186 stars 115 forks source link

Added data structure for exposing country descriptions #121

Open cweitkamp opened 4 years ago

cweitkamp commented 4 years ago

For discussion:

Added base classes for Countries, Regions and Cities.
Added prototype methods for requesting available countries, regions and cities.
The parsing - and optionally caching - could be improved. I just want to know if this is something which is in line with your library.

As a side note: May formatter is different than yours. Bear with me if this does not comply with your guidelines.

// EDIT: I just found your code-formatter.xml ...

Closes #110

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

coveralls commented 4 years ago

Coverage decreased (-0.8%) to 63.379% when pulling cb566db06bc7e1b15f5a6341852dfad6d2f433b0 on cweitkamp:feature-110-country-description-provider into fcc9b8f9b472fe5072f2944fbb32704c33f70074 on svendiedrichsen:master.

svendiedrichsen commented 4 years ago

@cweitkamp Thank you for your effort but this API is in my view only about handling holiday calendars. Those calendars are structuring holidays hierarchically without interpreting those hierarchies. They can be (and are mainly, looking at the provided ones) countries. But even the provided calendars contain examples like _Holidays_nsyeeuronext.xml which contain holidays of the NYSE. Generally should it be possible to have any kind of hierarchical structure of holidays like i.e.

one for an international company and its national sub-companies
or a company and some form of company holidays by company branches
a calendar of all stock exchanges worldwide
some kind of personal calendar of all family members

I admit that I have also started thinking only about countries first but since I have seen several different kind of holiday calendar structures.

cweitkamp commented 4 years ago

@svendiedrichsen Thank you for your opinion. You are absolutely right. To take care of the country structure is not the purpose of this API.

For the requirements specified in #110 it would be sufficient to expose the resource bundles of country descriptions. Would it be okay for you to change the visibility of the methods ResourceUtil::getCountryDescriptions() and ResourceUtil::getHolidayDescriptions() to public? The rest can be implemented on our side.

https://github.com/svendiedrichsen/jollyday/blob/fcc9b8f9b472fe5072f2944fbb32704c33f70074/src/main/java/de/jollyday/util/ResourceUtil.java#L162-L164