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

[roger riggs]

Hi Stephen,

I made the updates:

Updated Webrev:
     http://cr.openjdk.java.net/~rriggs/webrev-lenient-parse-218/

I did prefer it to use minWidth but earlier comments suggests being 
consistent
with SimpleDateFormat which is explicitly 2 digits.
The leniency for the sign character is separate from the leniency for the
field width.  At the point that ReducedPrinterParser is checking the width
it receives only the number of digits parsed.  So the current code does not
interpret +1 or -1 as reduced.

I also restructured the tests a bit to make it easier to see the strict
and lenient data expected values for a given input.

Thanks, Roger



On 5/1/2013 7:09 AM, Stephen Colebourne wrote:
> 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: