On Thu, 3 Feb 2022 23:29:54 GMT, Naoto Sato <na...@openjdk.org> wrote:
> Following the prior discussion [1], here is the PR for the subject > enhancement. CSR has also been updated according to the suggestion. > > [1] > https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-January/085175.html src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 5115: > 5113: > 5114: private LocalizedPrinterParser(FormatStyle dateStyle, > FormatStyle timeStyle, String requestedTemplate) { > 5115: this.dateStyle = dateStyle; Drop this constructor; it opens up the possibility of ambiguity between the modes. Keep only the two constructors with disjoint checking and initialization. src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 5151: > 5149: */ > 5150: private DateTimeFormatter formatter(Locale locale, Chronology > chrono) { > 5151: var dtStyle = dateStyle !=null || timeStyle != null; Switch to using requestedTemplate == null or != null as the discriminator. src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 5162: > 5160: > 5161: formatter = new > DateTimeFormatterBuilder().appendPattern(pattern).toFormatter(locale); > 5162: DateTimeFormatter old = > FORMATTER_CACHE.putIfAbsent(key, formatter); .computeIfAbsent(key, () -> {....}) might be a bit cleaner/clearer here and avoid the race a few lines of code below. (a slight improvement in old code). src/java.base/share/classes/sun/util/locale/provider/LocaleResources.java line 606: > 604: var matcher = VALID_SKELETON_PATTERN.matcher(skeleton); > 605: if (!matcher.matches()) { > 606: throw new IllegalArgumentException("Requested template is > invalid: " + requestedTemplate); The substitutions are not going to be visible in the exception message. That might make it harder to identify and fix the cause of the exception. Perhaps the message can make it clear that the matching was done after substitutions and show the 'skeleton'. ------------- PR: https://git.openjdk.java.net/jdk/pull/7340
