Handle combined holiday calendars correctly in HolidaySafeReferenceData #2426
Conversation
| @@ -280,7 +280,7 @@ public default int daysBetween(LocalDate startInclusive, LocalDate endExclusive) | |||
| * Combines this holiday calendar with another. | |||
| * <p> | |||
| * The resulting calendar will declare a day as a business day if it is a | |||
| * business day in both source calendars. | |||
| * business day in either source calendar. | |||
There was a problem hiding this comment.
I don't think this correction is correct. combinedWith is where a business day only happens if both calendars agree that it is a business day.
There was a problem hiding this comment.
The CombinedHolidayCalendar.isHoliday() method is calendar1.isHoliday(date) || calendar2.isHoliday(date) and the LinkedHolidayCalendar.isHoliday() method is calendar1.isHoliday(date) && calendar2.isHoliday(date) so I don't think this is correct? The changes here also line up with the existing JavaDoc for both CombinedHolidayCalendar and LinkedHolidayCalendar.
There was a problem hiding this comment.
The code refers to holidays, the Javadoc to business days.
There was a problem hiding this comment.
Ah okay, I did misread that, shall revert.
| //------------------------------------------------------------------------- | ||
| @Test | ||
| public void coverage() { | ||
| Map<ReferenceDataId<?>, Object> dataMap = ImmutableMap.of(ID1, VAL1); |
There was a problem hiding this comment.
Is this actually testing HolidaySafeReferenceData?
| return ImmutableHolidayCalendar.of(id, ImmutableList.of(), WEEKEND_DAYS); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
I would like to see tests using getValue or findValue in the super-interface as that is the main API that needs to be satisfied. eg. searching for GBLO+EUTA where GBLO is in reference data but EUTA is not should default EUTA and then combine it.
HolidayCalendar.combinedWith()andHolidayCalendar.linkedWith()HolidaySafeReferenceData, don't default to creating anImmutableHolidayCalenderwhenqueryValueOrNull()andtryDefaultValue()are called. If null is returned, thenHolidayCalendarId.create()will handle splitting the IDs appropriately and try to get the calendar using the individual calendar IDs