> 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 `getSpecialzied...
Nir Lisker has updated the pull request incrementally with two additional commits since the last revision: - review comments 2 - review comments 1 ------------- Changes: - all: https://git.openjdk.org/jfx/pull/1880/files - new: https://git.openjdk.org/jfx/pull/1880/files/2b610c6a..3448438b Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1880&range=01 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1880&range=00-01 Stats: 169 lines in 24 files changed: 18 ins; 5 del; 146 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
