<div dir="ltr">Alright, I'll document the current behavior.</div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Aug 19, 2025 at 1:43 AM Kevin Rushforth <<a href="mailto:kevin.rushforth@oracle.com">kevin.rushforth@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><u></u>

  
  <div>
    I am not sure it is worth changing the behavior either. It allowing
    null were ambiguous (or harmful), then it might be worth it. As it
    is, it seems better to just document the current behavior.<br>
    <br>
    -- Kevin<br>
    <br>
    <br>
    <div>On 8/18/2025 2:34 PM, John Hendrikx
      wrote:<br>
    </div>
    <blockquote type="cite">
      
      <p>I think it may come down to whether rejecting `null` would be
        helpful for users of these constructors to find problems in
        their code.  I agree that the DateTimeStringConverter is not
        really a chained constructor with a clear (and public) master
        constructor that is documented to allow `null` for certain
        defaults.  It is more a style of programming used throughout FX
        to provide many constructors (Image is also like that) instead
        of a Builder or static factory method.<br>
        <br>
        I could make arguments that it would make programmatic use
        easier, but that never stopped me from rejecting nulls in my own
        code.  Callers then should just pick their own defaults, like:<br>
        <br>
               new DateTimeConverter(locale == null ? Locale.DEFAULT :
        locale, pattern == null ? "ddMMyy" : pattern)<br>
        <br>
        Personally, I think this is not worth changing, but I do think
        it should be documented.  Rejecting nulls now will definitely
        require a CSR and is not backwards compatible :/<br>
        <br>
        --John<br>
        <br>
        On 18/08/2025 22:41, Nir Lisker wrote:</p>
      <blockquote type="cite">
        <div dir="ltr">
          <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">But
            I think I see what you mean better now, for example, there
            is <span style="background-color:rgb(255,255,255);padding:0px 0px 0px 2px"><span style="color:rgb(0,0,0);font-family:Consolas;font-size:11pt"><span>(Locale
                  locale, String pattern) </span></span></span>and <span style="background-color:rgb(255,255,255);padding:0px 0px 0px 2px"><span style="color:rgb(0,0,0);font-family:Consolas;font-size:11pt"><span>(String
                  pattern)</span></span></span>. If you intending to
            pass `null` as Locale, why not just use the pattern-only
            constructor?  I think that still could make sense,</blockquote>
          <div><br>
          </div>
          <div>OK, but what about `pattern` being null in either?</div>
        </div>
        <br>
        <div class="gmail_quote">
          <div dir="ltr" class="gmail_attr">On Mon, Aug 18, 2025 at
            11:30 PM John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank">john.hendrikx@gmail.com</a>>
            wrote:<br>
          </div>
          <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
            <div>
              <div>If certain patterns make no sense, then I think the
                master constructor should reject them (if there is one,
                or perhaps one should be created as a private one).</div>
              <div><br>
              </div>
              <div>For example, if there is a width/height constructor,
                but I only specify width and not height, then I think
                its fine to reject (null, height) and (width, null)
                while accepting (null, null) and (width, height).</div>
              <div><br>
                But I think I see what you mean better now, for example,
                there is <span style="background-color:rgb(255,255,255);padding:0px 0px 0px 2px"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:Consolas;font-size:11pt;white-space:pre-wrap"><span style="color:rgb(0,0,0)">(Locale locale, String pattern) </span></span></span>and
                <span style="background-color:rgb(255,255,255);padding:0px 0px 0px 2px"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:Consolas;font-size:11pt;white-space:pre-wrap"><span style="color:rgb(0,0,0)">(String pattern)</span></span></span>.
                If you intending to pass `null` as Locale, why not just
                use the pattern-only constructor?  I think that still
                could make sense, especially locale is say coming from a
                configuration file or some such.  If the constructor
                `null` here, then it makes it easier to use it
                programmatically (ie. if my configuration has a Locale
                then I pass that, if it is null, I pass that -- compare
                that to having to check for Locale being null and then
                having to select the correct constructor...</div>
              <div><br>
              </div>
              <div>--John<br>
              </div>
              <div><br>
              </div>
              <div>On 18/08/2025 21:00, Nir Lisker wrote:<br>
              </div>
              <blockquote type="cite">
                <div dir="ltr">
                  <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Aren't
                    these constructors chained?</blockquote>
                  <div><br>
                  </div>
                  <div>Not all of them. There are 3 tangential creation
                    paths for DateTimeStringConverter and subclasses:</div>
                  <div>1. Through dateStyle/timeStyle ints with an
                    optional locale that creates the DateFormat with
                    `DateFormat.getDateTimeInstance(dateStyle,
                    timeStyle, locale)` (similar for subclasses). There
                    are 4 constructors here for the combinations of:
                    none, only style(s), only locale, all.</div>
                  <div>2. Through a String pattern with an optional
                    locale that creates the DateFormat with `new
                    SimpleDateFormat(pattern, locale)`. If pattern is
                    null, it uses path 1 above with default styles and
                    the optional locale. If you wanted to use a pattern
                    to create this converter, passing null makes little
                    sense. It could be surprising to get a
                    "defaults" converter when you intended to
                    customize the format with your own pattern to
                    begin with. I imagine that if you couldn't get a
                    pattern, you'd use your own int styles as a
                    replacement.<br>
                    <div>3. Through a DateFormat that is used directly.
                      If it's null, it checks if the pattern is null
                      (which it will always be), in which case it uses
                      the defaults of path 1 again (with the default
                      locale since it's not optional here). I find it
                      odd here too to pass null and rely on the
                      "defaults" converter.</div>
                    <div><br>
                    </div>
                    <div>NumberStringConverter and its subclasses are
                      similar. They have path 2 with 4 combinations:
                      none, only pattern, and locale, and both. And they
                      also have path 3 that falls on the default if
                      null.</div>
                  </div>
                </div>
                <br>
                <div class="gmail_quote">
                  <div dir="ltr" class="gmail_attr">On Mon, Aug 18, 2025
                    at 9:06 PM John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank">john.hendrikx@gmail.com</a>>
                    wrote:<br>
                  </div>
                  <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                    <div>
                      <p>Aren't these constructors chained?  I believe
                        it is quite common practice to use nulls when
                        calling a chained constructor to get a default
                        for that parameter.  If a certain type of
                        convenience constructor is missing, a caller
                        can pass in `null` for the parameter they'd like
                        defaulted.  It's not too far-fetched to allow
                        this **if** there is a constructor where this
                        parameter is omitted and is assigned a default.<br>
                        <br>
                        If anything, the constructors IMHO should
                        document that for certain parameters passing in
                        `null` results in a specific default.<br>
                      </p>
                      <p>--John<br>
                      </p>
                      <div>On 18/08/2025 19:46, Nir Lisker wrote:<br>
                      </div>
                      <blockquote type="cite">
                        <div dir="ltr">Hi all,<br>
                          <br>
                          In DateTimeStringConverter,
                          NumberStringConverter, and their subclasses,
                          null parameters sent to the constructors are
                          internally converted to default values. This
                          is not specified, but it's how the
                          implementation behaves. I'm working on some
                          changes there and was thinking about changing
                          the behavior to throw NPEs as it makes
                          no sense to pass null into these and it's
                          probably a bug more than anything.
                          <div><span style="background-color:rgb(0,128,0);color:rgb(18,144,195);font-family:Consolas;font-size:11pt;white-space:pre-wrap"></span></div>
                          <div><br>
                          </div>
                          <div>The LocalDate/Time converters specified
                            the null-friendly behavior in their docs
                            even though it doesn't make much sense there
                            either.</div>
                          <div><br>
                          </div>
                          <div>Are we allowed to change this?</div>
                          <div><br>
                          </div>
                          <div>- Nir</div>
                        </div>
                      </blockquote>
                    </div>
                  </blockquote>
                </div>
              </blockquote>
            </div>
          </blockquote>
        </div>
      </blockquote>
    </blockquote>
    <br>
  </div>

</blockquote></div>