Skip to content

fix: default null scientific format option to false - #1078

Open
skytin1004 wants to merge 4 commits into
apache:mainfrom
skytin1004:fix/data-formatter-scientific-option-default
Open

fix: default null scientific format option to false#1078
skytin1004 wants to merge 4 commits into
apache:mainfrom
skytin1004:fix/data-formatter-scientific-option-default

Conversation

@skytin1004

@skytin1004 skytin1004 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Purpose of the pull request

While reviewing my formatter cache changes in #1068, I noticed a bug in the DataFormatter constructor when either formatting option is null.

For example, new DataFormatter(false, Locale.US, null) should use the default of false for scientific notation. Before this fix, formatting 100000000000 with the General format throws a NullPointerException. After this fix, it returns 100000000000 as expected.

Another example is new DataFormatter(null, Locale.US, true). Before this fix, formatting the same number returns 100000000000, even though scientific notation is enabled. After this fix, it returns 1E+11.

The constructor was checking use1904windowing instead of useScientificFormat when applying the scientific-notation default. This PR changes that one condition. The examples call the constructor directly because NumberDataFormatterUtils already converts null options to their defaults after #1068.

What's changed?

  • Fix the condition to check useScientificFormat, preserving its documented default of false.
  • Add a parameterized constructor regression test covering null, enabled, and disabled scientific-format options, including null date-windowing options.

Verification:

All 919 fesod-sheet tests passed on JDK 11 with the default locale set to Korean, and the build completed successfully.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@skytin1004
skytin1004 marked this pull request as ready for review September 8, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant