On Mon, 17 Jun 2024 18:17:06 GMT, Justin Lu <j...@openjdk.org> wrote:
>>> I second Justin's suggestion here. The change should benefit every >>> implementation in the JDK, not only NumberFormat. Also, I might suggest >>> renaming the new class, as it is kind of long/redundant. How about using >>> something as simple as "StringBuf"? >> >> Thanks for your comment. The long name bother me for a while. I have changed >> it to ""StringBuf". > > Hi @lingjun-cg > > Thanks for your work here. > > While this would benefit the performance of NumberFormat subclasses, we are > hoping that if we are going to make the internal switch to StringBuilder, we > can also make similar changes to other Format subclasses as well where > applicable. So, we would want `isInternalSubclass()` to be defined at the > Format level. Some methods that I can immediately think of would be `String > DateFormat.format(Date date)`, `String ListFormat.format(List<String> input)` > and, `String MessageFormat.format(Object obj)`. > > I believe @naotoj shares the same sentiment. @justin-curtis-lu The method `isInternalSubclass` in `DecimalFormat` already pulls up to `Format`. @liach @naotoj Using `Appendable & CharSequence` instead of an interface with a limited method is a great idea. Following this idea, I have updated the PR. But there are some notes when implementing the idea. 1. Rename `format` to `formatWithGeneric` in DecimalFormat that accepts StringBuilder. If not that, throw a StackOverflowException when run with `DecimalFormat.format(double)`. If not rename the format, there are 2 methods in DecimalFormat: ``` @Override public StringBuffer format(double number, StringBuffer result, FieldPosition fieldPosition) { return format(number, result, fieldPosition); } @Override <T extends Appendable & CharSequence> T format(double number, T result, FieldPosition fieldPosition) { } Because the second parameter type `StringBuffer` of the 1st method is more concrete than type `T` of the 2nd method, so the 1st method will call itself recursively. 2. The `Appendable#append(java.lang.CharSequence)` method can throws IOException, so there is some messy code handling IOException. ------------- PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2175778275