<i18n dev> RFR: JDK-8316435: sun.util.calendar.CalendarSystem subclassing should be restricted

[Naoto Sato]

On Mon, 18 Sep 2023 22:42:09 GMT, Justin Lu <jlu at openjdk.org> wrote:

> Please review this PR which restricts sub-classing of the internal calendar system in sun.util.calendar to only the existing implementations.
> 
> As the implementation is long-standing and complete with no intent for future sub-classing, the CalendarSystem should be made sealed. Modifiers adjusted accordingly (`JulianCalendar.Date` must now have package visibility).
> 
> 
> This system has the following structure,
> 
> `CalendarSystem` extended by `AbstractCalendar` extended by `BaseCalendar` extended by 
> (`Gregorian, JulianCalendar, LocalGregorianCalendar`)
> 
> `CalendarDate` extended by `BaseCalendar.Date` extended by 
> (`Gregorian.Date, ImmutableGregorianDate, JulianCalendar.Date, LocalGregorianCalendar.Date`)
> 
> Additionally, CalendarUtils was made `final`, as it is a utility class composed of static util methods.

Looks good overall.

src/java.base/share/classes/sun/util/calendar/CalendarUtils.java line 30:

> 28: public final class CalendarUtils {
> 29: 
> 30:     private CalendarUtils() {}

It may be obvious, but adding a comment would be helpful, i.e., no instance may be created.

src/java.base/share/classes/sun/util/calendar/CalendarUtils.java line 132:

> 130:      * Mimics sprintf(buf, "%0*d", decaimal, width).
> 131:      */
> 132:     public static StringBuilder sprintf0d(StringBuilder sb, int value, int width) {

Can this (and the following overload) method be eliminated? No need to mimic `sprintf` here, but using `String.format`/`formatted` should suffice?

src/java.base/share/classes/sun/util/calendar/Gregorian.java line 40:

> 38: 
> 39:     static final class Date extends BaseCalendar.Date {
> 40:         Date() {

I think removing `protected` from the `final` class methods is fine. Adding `@Override` may be helpful.

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

PR Review: https://git.openjdk.org/jdk/pull/15803#pullrequestreview-1633738951
PR Review Comment: https://git.openjdk.org/jdk/pull/15803#discussion_r1330414349
PR Review Comment: https://git.openjdk.org/jdk/pull/15803#discussion_r1330430127
PR Review Comment: https://git.openjdk.org/jdk/pull/15803#discussion_r1330418996