Exposed implementation details in DateTimeStringConverter

[Kevin Rushforth]

I presume you mean these three?

     protected final Locale locale;
     protected final String pattern;
     protected final DateFormat dateFormat;

and maybe also these two?

     /**
      * @since JavaFX 8u40
      */
     protected final int dateStyle;

     /**
      * @since JavaFX 8u40
      */
     protected final int timeStyle;

Applications can't use them directly (outside a subclass), so the 
questions are: Do subclasses need access to these fields, and do we 
intend that an application be able to subclass the parent class with 
their own implementation?

The fields are used by the two subclasses that we provide in the same 
package, so unless we expect applications or third-party libraries to 
access them, which I don't think we do, it doesn't seem like they should 
be part of the public API.

We should either file a bug to document them or to deprecate them 
(probably for removal, but I wouldn't be in too big a big hurry to 
remove them). I suspect deprecating them is fine.

-- Kevin


On 1/26/2021 12:08 PM, Nir Lisker wrote:
> Hi,
>
> DateTimeStringConverter exposes final protected fields that are used for
> internal implementation. These fields should have been private or
> package-private. The reasons are:
> 1. A comment above them refers to them as private fields.
> 2. They have no documentation attached.
> 3. They are useless to the user (except for the DateFormat one) as some of
> their values are unused, depending on the construction method, and they are
> discarded after it.
> 4. There is a DateFormat getter method specifically for this field only,
> which is the only field that is relevant during the life of the class. It
> makes no sense to expose the field in addition.
>
> I suggest deprecating these fields for removal from the public API. There
> are minimal risks that these fields are being read by the user through
> direct access.
>
> - Nir