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
