<i18n dev> RFR: 8225641: Calendar.roll(int field) does not work correctly for WEEK_OF_YEAR [v5]

[Justin Lu]

On Wed, 29 Mar 2023 23:14:17 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Justin Lu has updated the pull request incrementally with five additional commits since the last revision:
>> 
>>  - Impl cleanup, add Saturday end day conditional
>>  - Rename test, clarify test documentation
>>  - Add type to static declaration of Cal.Builder, since only concerned with Gregorian for this test
>>  - Pass in amount as 1 directly
>>  - Reuse Calendar.Builder
>
> src/java.base/share/classes/java/util/GregorianCalendar.java line 3014:
> 
>> 3012:      */
>> 3013:     private boolean dayInMinWeek (int day, int startDay, int endDay) {
>> 3014:         endDay = endDay == 0
> 
> Why `endDay` can be `0`? If the arguments are `Calendar.XXXDAY`, should they be between 1 to 7? In other words, why is the above calling site doing `getFirstDayOfWeek() - 1`?
> I'd add some range checks for the input values.

Right. I have changed it so the method throws an exception if the start and end days are not valid `DAY_OF_WEEK `values.

And I have moved the ternary of switching 0 to 7 outside the method accordingly.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13031#discussion_r1153805524