[threeten-dev] TzdbZonRulesProvider and providerBind

[Stephen Colebourne]

Looks good to me.

It would be OK to remove line 129, the first line of
provideRules(String zoneId) that checks for null, as the map lookup
will throw NPE anyway being a ConcurrentHashMap.

Stephen


On 29 January 2013 19:24, Xueming Shen <xueming.shen at oracle.com> wrote:
> The webrev has been updated.
>
> http://cr.openjdk.java.net/~sherman/jdk8_threeten/tzdbProvider
>
>
>
> On 01/29/2013 11:20 AM, Stephen Colebourne wrote:
>>
>> Are you awaiting a review on this? Please re-send links if you want a
>> review.
>> Stephen
>>
>> On 29 January 2013 16:47, Xueming Shen<xueming.shen at oracle.com>  wrote:
>>>
>>> On 1/29/13 2:30 AM, Stephen Colebourne wrote:
>>>>
>>>> On 29 January 2013 07:54, Xueming Shen<xueming.shen at oracle.com>  wrote:
>>>>>
>>>>> A slim down version of the tzdbProvider.java
>>>>>
>>>>> http://cr.openjdk.java.net/~sherman/jdk8_threeten/tzdbProvider
>>>>
>>>> There should be a blank line before the constructor.
>>>>
>>>> The "Specification for implementors" section is irrelevent on a
>>>> non-public class. The class is also not immutable.
>>>>
>>>> The map of versions shoud use Collections.singletonMap or emptyMap
>>>> (note that this requires changing the specification in
>>>> ZoneRulesProvider)
>>>
>>>
>>> Thought about that last night, then decided not to touch the spec for
>>> now.
>>> We will need to go through the ZoneRulesProvider to decide if the
>>> returned
>>> map should be immutable or not all together, the getAvailableZoneId as
>>> well.
>>> It probably should never hit the emptyMap, given it's
>>> id->provider->versions(id)
>>> invocation.
>>>
>>> The webrev has been updated with other suggestions.
>>>
>>> -Sherman
>>>
>>>
>>>
>>>> I think that the version should probably be added to the toString() -
>>>> "TZDB[2013a]"
>>>>
>>>> A comment should be added noting that only the latest version is being
>>>> used.
>>>>
>>>>
>>>> In general, this is a good simplification.
>>>>
>>>> Stephen
>>>
>>>
>