[threeten-dev] ZoneIdPrinterParser #262

[Stephen Colebourne]

ZoneId has a meaningless if statement now line 473.

You fixed the third Javadoc example in DTFBuilder, but not the first
two, line 964 and 1014

TCKZoneId has a mispelling - "nullPrefixOfOffste"
It should also have a test for a valid prefix but a null offset.

Otherwise looks good.
thanks
Stephen



On 10 June 2013 22:08, roger riggs <roger.riggs at oracle.com> wrote:
> Hi,
>
> Updated the webrev to use a new ZoneId.ofOffset(prefix, zoneOffset) method.
> Corrected some tests that still allowed UT0, UTC0, GMT0 and the prior forms
> of ZoneIds.  This change also affects the default parser of ZoneDateTime.
> The results do match ZoneId.of.
>
> http://cr.openjdk.java.net/~rriggs/webrev-zoneid-parser-262/
>
> Please review and comment.
>
> Thanks, Roger
>
>
>
>
> On 6/10/2013 11:35 AM, Stephen Colebourne wrote:
>>
>> On 8 June 2013 18:34, roger riggs <roger.riggs at oracle.com> wrote:
>>>
>>> Please review the changes and comment whether the test cases are correct.
>>> There are a number of input values that were previously valid for parsing
>>> of ZoneIds that are no longer valid (to match ZoneId).  For example,
>>> "UTC0"
>>> is no longer valid.
>>
>> Looks right to me. It may be worth a cross-test with ZoneId.of to
>> ensure both work the same.
>>
>>> One inconvenience in the code after a prefix and the offset value is
>>> parsed
>>> there is no public method to create the ZoneId.  The ZoneId.of method as
>>> to
>>> re-parse the input.
>>> The method   static ZoneRegion ofPrefixedOffset(String zoneId, ZoneOffset
>>> offset)
>>> is use for this purpose but it is package private and can't be used from
>>> the
>>> format package.
>>> Is it reasonable to add a method to ZoneId for a similar purpose?
>>
>> I think adding a ZoneId.ofOffset(String prefix, ZoneOffset offset)
>> method would be OK. I don't think it needs a longer name than
>> "ofOffset" because the arguments and javadoc should be clear enough. I
>> think that as well as "UTC, "UT" and "GMT" prefixes, it should accept
>> a blank string, which just returns the offset.
>>
>> thanks
>> Stephen
>>
>>
>>> http://cr.openjdk.java.net/~rriggs/webrev-zoneid-parser-262/
>>>
>>> Thanks, Roger
>>>
>>>
>>>
>