sahvx655-wq commented on code in PR #408:
URL: https://github.com/apache/commons-validator/pull/408#discussion_r3479510923
##########
src/test/java/org/apache/commons/validator/routines/CalendarValidatorTest.java:
##########
@@ -232,6 +232,27 @@ void testCompare() {
assertEquals("Invalid field: -1", e.getMessage(), "check message");
}
+ /**
+ * Test compareWeeks() at the week-year boundary, where WEEK_OF_YEAR is
relative to a
+ * week-year that differs from the calendar year.
+ */
+ @Test
+ @DefaultLocale(country = "US", language = "en")
+ void testCompareWeeksAcrossYearBoundary() {
+ final int noon = 120000;
+ // 31 Dec 2018 (Monday) is in week 1 of week-year 2019 (US: first day
Sunday, minimal days 1)
+ final Calendar dec31y2018 = createCalendar(TimeZones.GMT, 20181231,
noon);
+ final Calendar jan01y2018 = createCalendar(TimeZones.GMT, 20180101,
noon);
+ final Calendar jan01y2019 = createCalendar(TimeZones.GMT, 20190101,
noon);
+
+ // both are calendar year 2018 and week-of-year 1, but lie ~52 weeks
apart
+ assertEquals(1, calValidator.compareWeeks(dec31y2018, jan01y2018),
"Dec 31 2018 is weeks after Jan 1 2018");
+ assertEquals(-1, calValidator.compareWeeks(jan01y2018, dec31y2018),
"Jan 1 2018 is weeks before Dec 31 2018");
+
+ // 31 Dec 2018 and 1 Jan 2019 share week 1 of week-year 2019, so they
are the same week
+ assertEquals(0, calValidator.compareWeeks(dec31y2018, jan01y2019),
"Dec 31 2018 is the same week as Jan 1 2019");
+ }
+
Review Comment:
The new version doesn't call isWeekDateSupported() or getWeekYear() any
more, so that branch is gone. It keys off the day distance and the week number
instead, which avoids the unsupported-calendar case entirely. I've added
boundary tests for WEEK_OF_YEAR and WEEK_OF_MONTH in its place.
##########
src/main/java/org/apache/commons/validator/routines/AbstractCalendarValidator.java:
##########
@@ -416,4 +422,14 @@ protected Object parse(String value, final String pattern,
final Locale locale,
*/
@Override
protected abstract Object processParsedValue(Object value, Format
formatter);
+
+ /**
+ * Returns the week-year of a calendar, which a {@code WEEK_OF_YEAR} value
is relative to.
+ *
+ * @param calendar The calendar to read the week-year from.
+ * @return the week-year, or the calendar year if week dates are not
supported.
+ */
+ private int weekYear(final Calendar calendar) {
+ return calendar.isWeekDateSupported() ? calendar.getWeekYear() :
calendar.get(Calendar.YEAR);
+ }
Review Comment:
Fair point, that YEAR fallback was effectively turning the comparison into a
year comparison. I've dropped weekYear() altogether. compareWeek() now compares
the day gap first and only consults the week number when the dates are under a
week apart, so there's nothing left to fall back from.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
