[threeten-dev] Review for parsing 2 to 4 years #218

[Stephen Colebourne]

DTFB
"If the number of characters parsed is two and the value is positive,"
I guess this should be something more like
"If the number of characters equals the width and all character are
digits" (ie. no signs)
because neither +1 nor -1 should be accepted as a reduced-base-value.
(this comment applies to both reduced methods)

The second reduced method's first argument is "minWidth", but perhaps
it should be "width" (and the other maxParseWidth?). This would make
sense wrt the first reduced method. For full control over all the
widths, you actually need three arguments.

Looking at the code, it looks like there is a hard coded width of 2
"if parseLen == 2 && value >= 0)", but it should depend on the input
width. Or am I reading it wrong.
This would affect these tests:
{YEAR, 1, 2010, "1", 0, 1, 1},  should be year 2011
{YEAR, 1, 2010, "3", 1, 1, 3},  should be year 2013
{YEAR, 1, 2010, "9", 0, 1, 9},  should be year 2019
{YEAR, 1, 2005, "0", 0, 1, 0},  should be year 2010
{YEAR, 1, 2005, "4", 0, 1, 4},  should be year 2014
{YEAR, 1, 2005, "5", 0, 1, 5},  should be year 2005
{YEAR, 1, 2005, "9", 0, 1, 9},  should be year 2009
and tests should be added for width 3.
other tests will be affected too.

thanks
Stephen



On 30 April 2013 20:17, roger riggs <roger.riggs at oracle.com> wrote:
> Thanks  for the detailed review.
>
> Updated Webrev:
>     http://cr.openjdk.java.net/~rriggs/webrev-lenient-parse-218/
>
> Javadoc:
>     http://cr.openjdk.java.net/~rriggs/javadoc-lenient-parse-218/
>
>
>
> On 4/30/2013 2:24 AM, Stephen Colebourne wrote: