[threeten-dev] [threeten-develop] DateTimeFormatters factory consolidation #210

[Roger Riggs]

Thanks for the review.

On 1/28/2013 6:03 PM, Stephen Colebourne wrote:
> On DateTimeFormatter you changed "This class is immutable and
> thread-safe." to "All returned formatters are immutable and
> thread-safe." In fact, both statements are true, but the first is
> probably more true. (Given that SimpleDateFormat is not thread-safe,
> this one is important to get right)
fixed  (I had merged the specification for implementors from the 
DateTimeFormatters class).
>
> It might be worth having an inner class lazy load for some/all of the
> constants. The RFC constant will be slow to create with its maps and
> rarely used. The other constants might be better having their startup
> delayed as well.
I would agree about delaying the initialization if possible but
with static variables, I don't think the initialization can be lazy.

It might be possible for the implementation to instantiate a 
DateTimeFormatter instance
with only an enum of its format but not initialize the fields.  I'll 
take a look at that after
other tasks.

It also occurs to me that some folks would benefit of documentation of 
the pattern
used. It might provoke better recognition in addition to the name.  But 
it is not always
possible.  Thing for a future doc update perhaps.

Thanks, Roger

>
> Rest of the change looks fine.
>
> Stephen
>
>
> On 28 January 2013 22:25, Roger Riggs<Roger.Riggs at oracle.com>  wrote:
>> Please review:
>>
>> webrev:
>>      http://cr.openjdk.java.net/~rriggs/webrev-factory-210/
>>
>> javadoc:
>>      http://cr.openjdk.java.net/~rriggs/javadoc-factory-210/
>>
>> The factory methods are merged from DateTimeFormatters to DateTimeFormatter
>> and renamed as per conventions.
>>
>> Thanks, Roger
>>
>>