> > But I think I see what you mean better now, for example, there is (Locale > locale, String pattern) and (String pattern). If you intending to pass > `null` as Locale, why not just use the pattern-only constructor? I think > that still could make sense,
OK, but what about `pattern` being null in either? On Mon, Aug 18, 2025 at 11:30 PM John Hendrikx <john.hendr...@gmail.com> wrote: > 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). > > 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). > > But I think I see what you mean better now, for example, there is (Locale > locale, String pattern) and (String pattern). 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... > > --John > > On 18/08/2025 21:00, Nir Lisker wrote: > > Aren't these constructors chained? > > > Not all of them. There are 3 tangential creation paths for > DateTimeStringConverter and subclasses: > 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. > 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. > 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. > > 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. > > On Mon, Aug 18, 2025 at 9:06 PM John Hendrikx <john.hendr...@gmail.com> > wrote: > >> 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. >> >> If anything, the constructors IMHO should document that for certain >> parameters passing in `null` results in a specific default. >> >> --John >> On 18/08/2025 19:46, Nir Lisker wrote: >> >> Hi all, >> >> 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. >> >> The LocalDate/Time converters specified the null-friendly behavior in >> their docs even though it doesn't make much sense there either. >> >> Are we allowed to change this? >> >> - Nir >> >>
