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 >
