> Refactoring of all `StringConverter`s and their tests. General notes: > * The documentation language has been unified and `null` parameter rules have > been documented. > * Tests have been cleaned up in the vein of > https://github.com/openjdk/jfx/pull/1759 and unneeded `@BeforeAll`s were > removed. > * Internal fields were made `private final` to guarantee immutability. > > Incremental commits are provided for easier reviewing: > > ### Parent classes > * `StringConverter`: updated documentation > * `BaseStringConverter`: a new internal class that implements repeated code > from converter implementations and serves as an intermediate superclass. It > does empty and `null` string checks that are handled uniformly, except for > `DefaultStringConverter`, which has a different formatting mechanism. > > ### Primitive-related converters > * All primitive (wrapper) converters also document their formatting and > parsing mechanism since these are "well-established". > > ### Format converter > * Checked for `null` during constriction time to avoid runtime NPEs. > * There is no test class for this converter. A followup might be desirable. > * A followup should deprecate for removal `protected Format getFormat()` (as > in [JDK-8314597](https://bugs.openjdk.org/browse/JDK-8314597) and > [JDK-8260475](https://bugs.openjdk.org/browse/JDK-8260475)). > > ### Number and subclasses converters > * The intermediate `locale` and `pattern` fields were removed (along with > their tests). The class generated a new formatter from these on each call. > This only makes sense for mutable fields where the resulting formatter can > change, but here the formatter can be computed once on construction and > stored. > * The only difference between these classes is a single method for creating a > format from a `null` pattern, which was encapsulated in the > `getSpecializedNumberFormat` method. > * The terminally deprecated `protected NumberFormat getNumberFormat()` was > removed. Can be split to its own issue if preferred. In my opinion, it > shouldn't exist even internally since testing the internal formatter doesn't > help. The only tests here should be for to/from strings, and these are > lacking. A followup can be filed for adding more conversion tests. > > ### Date/Time converters > * Added a documentation note advising users to use the `java.time` classes > instead of the old `Date` class. > * As with Number converters, only the `dateFormat` field was kept, which is > created once on construction instead of on each call. > * As with Number converters, the `getSpecialzie...
Nir Lisker has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 14 additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into 8250802_Refactor_StringConverter_and_its_subclasses - Formatting fixes - Clarify thread safety - Merge master - Fix typo - Removed super() invocations - review comments 2 - review comments 1 - LocalDate/Time converters - Date/Time converters - ... and 4 more: https://git.openjdk.org/jfx/compare/3b276a28...fea3e36d ------------- Changes: - all: https://git.openjdk.org/jfx/pull/1880/files - new: https://git.openjdk.org/jfx/pull/1880/files/5beac1b3..fea3e36d Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1880&range=06 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1880&range=05-06 Stats: 650539 lines in 10025 files changed: 309580 ins; 241695 del; 99264 mod Patch: https://git.openjdk.org/jfx/pull/1880.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1880/head:pull/1880 PR: https://git.openjdk.org/jfx/pull/1880
