> 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 11 additional commits since the 
last revision:

 - Merge master
 - Fix typo
 - Removed super() invocations
 - review comments 2
 - review comments 1
 - LocalDate/Time converters
 - Date/Time converters
 - Number and subclasses converters
 - Format converter
 - Primitive-related converters
 - ... and 1 more: https://git.openjdk.org/jfx/compare/2e6d2313...a030ce58

-------------

Changes:
  - all: https://git.openjdk.org/jfx/pull/1880/files
  - new: https://git.openjdk.org/jfx/pull/1880/files/ffad73e5..a030ce58

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=1880&range=03
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1880&range=02-03

  Stats: 154009 lines in 1240 files changed: 84543 ins; 27129 del; 42337 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

Reply via email to