On Mon, 22 May 2023 23:03:10 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Please review this PR which updates the Scientific Notation section of >> Decimal Format. It aims to resolve >> [JDK-8159023](https://bugs.openjdk.org/browse/JDK-8159023) as well as >> [JDK-6282188](https://bugs.openjdk.org/browse/JDK-6282188). >> >> **Scientific Notation** in Decimal Format contains the definition for a >> scientific notation formatted number's mantissa as such: _The number of >> significant digits in the mantissa is the sum of the minimum integer and >> maximum fraction digits, and is unaffected by the maximum integer digits. >> For example, 12345 formatted with "##0.##E0" is "12.3E3"._ >> >> Both the definition and example are incorrect, as the actual result is >> "12.E345". >> >> The following example data show that scientific notation formatted numbers >> do not adhere to that definition. As, according to the definition, the >> following numbers should have 3 significant digits, but in reality, they >> have up to 5. >> >> 123 formatted by ##0.#E0 is 123E0 >> 1234 formatted by ##0.#E0 is 1.234E3 >> 12345 formatted by ##0.#E0 is 12.34E3 >> 123456 formatted by ##0.#E0 is 123.5E3 >> 1234567 formatted by ##0.#E0 is 1.235E6 >> 12345678 formatted by ##0.#E0 is 12.35E6 >> 123456789 formatted by ##0.#E0 is 123.5E6 >> >> >> The actual definition of the mantissa, as well as examples and further >> context are included in this change. In addition, a test has been added that >> tests various patterns to numbers and ensures the format follows the new >> definition's mathematical expression. > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > Review: Use Static dfs, error message, and exponent digit. Hard code min > and max digits instead of calculating at runtime Thanks, Justin. Left some super-nits. test/jdk/java/text/Format/DecimalFormat/MantissaDigits.java line 52: > 50: 1.1234, 1.1111, 1.412, 222.333, -771.2222 > 51: }; > 52: private static final DecimalFormatSymbols dfs = new > DecimalFormatSymbols(Locale.US); The `static final` field is better uppercased. test/jdk/java/text/Format/DecimalFormat/MantissaDigits.java line 64: > 62: for (double number : numbers) { > 63: // Count the significant digits in the pre-formatted number > 64: int originalNumDigits = String.valueOf(number) It's OK as it stands, but you could write: str.chars().filter(Character::isDigit).count() which might be more readable ------------- PR Review: https://git.openjdk.org/jdk/pull/14066#pullrequestreview-1437867369 PR Review Comment: https://git.openjdk.org/jdk/pull/14066#discussion_r1201295121 PR Review Comment: https://git.openjdk.org/jdk/pull/14066#discussion_r1201295601
