Please Review fix for reduced value parser 8024076
roger riggs
roger.riggs at oracle.com
Wed Oct 2 18:41:22 UTC 2013
Hi Sherman,
Thanks for the detailed review, the webrev has been updated with your
recommendation.
Roger
On 10/2/2013 1:50 PM, Xueming Shen wrote:
> On 10/02/2013 10:20 AM, roger riggs wrote:
>> Hi Sherman,
>>
>> The BASE_DATE is the default ChronoLocalDate and is used outside of RPP.
>> RPP itself uses any ChronoLocalDate, not the specific one.
>> Scoping BASE_DATE to RPP would not delay the initialization
>> since it would need to be initialized at line 1715.
>
> The BASE_DATE will not be initialized until the code runs into
> line#1715, if
> we hide the BASE_DATE into RPP, which might benefit applications that
> never uses the "reducedValue" functionality. The BASE_DATE is something
> only used for appendValueReduced() functionality and RPP is its
> implementation,
> so I don't see any probably logically to move it into RPP, maybe
> rename it
> to ISO_BASE_DATE...
>
> -Sherman
>
>
>>
>> Roger
>>
>>
>>
>> On 10/2/2013 12:54 PM, Xueming Shen wrote:
>>> Should move the static field BASE_DATE into ReducePrinterParser?
>>> Logically (and for performance, if it matters at all) RPP appears to
>>> be a better place for this constant.
>>>
>>> The rest looks fine.
>>>
>>> -Sherman
>>>
>>> On 10/02/2013 08:19 AM, roger riggs wrote:
>>>> Please review this fix for parsing two digit years in an Chronology.
>>>>
>>>> The webrev includes Stephen's proposed alternate method that provides
>>>> a ChronoLocalDate as the base date.
>>>>
>>>> http://cr.openjdk.java.net/~rriggs/webrev-two-digit-8024076/
>>>>
>>>> Thanks, Roger
>>>>
>>>> p.s. the design issue raised in the comments has been filed as [2]
>>>> : 8025828
>>>>
>>>> [1]https://bugs.openjdk.java.net/browse/JDK-8024076
>>>> [2]https://bugs.openjdk.java.net/browse/JDK-8025828
>>>>
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list