Review request for IMF classes and Locale related classes

[Naoto Sato]

OK here is the revised one:

http://cr.openjdk.java.net/~naoto/7117469/webrev.02/

I hope this is the final :-)

Naoto

On 12/2/11 5:41 PM, Stuart Marks wrote:
> The changes you made are good, thanks.
>
> I did figure out how to deal with the static initializer in
> AllAvailableLocales:
>
> @SuppressWarnings({"unchecked"})
> Class<? extends LocaleServiceProvider>[] providerClasses =
> (Class<? extends LocaleServiceProvider>[]) new Class<?>[] {
> java.text.spi.BreakIteratorProvider.class,
> ...
> };
>
> This is a bit uglier but it does reduce the scope of warnings suppression.
>
> I think you might be able to get rid of the wildcards, too:
>
> @SuppressWarnings({"unchecked"})
> Class<LocaleServiceProvider>[] providerClasses =
> (Class<LocaleServiceProvider>[]) new Class<?>[] {
> java.text.spi.BreakIteratorProvider.class,
> ...
> };
> ...
> for (Class<LocaleServiceProvider> providerClass : providerClasses) {
> ...
> }
>
> I'm not entirely sure why this works, but it seems to compile for me
> without warnings or errors....
>
> s'marks
>
>
>
> On 12/2/11 4:00 PM, Naoto Sato wrote:
>> Hi Stuart,
>>
>> Thank you for your comments. Here is the updated webrev reflecting your
>> suggestions. Please review:
>>
>> http://cr.openjdk.java.net/~naoto/7117469/webrev.01/
>>
>> Naoto
>>
>>
>> On 12/2/11 3:32 PM, Stuart Marks wrote:
>>> Hi Naoto,
>>>
>>> A couple comments.
>>>
>>> java/util/Currency.java --
>>>
>>> The @SuppressWarnings covers the entire method. We're trying to use
>>> @SuppressWarnings with as narrow a scope as possible. Sometimes it's
>>> helpful to create a local variable declaration for this purpose; perhaps
>>> something like this will help:
>>>
>>> @SuppressedWarnings("unchecked")
>>> Set<Currency> result = (Set<Currency>) available.clone();
>>> return result;
>>>
>>> You can probably do something similar in
>>> sun/util/LocaleServiceProviderPool.java in the getLocalizedObjectImpl()
>>> method.
>>>
>>> The suppression of warnings on the static AllAvailableLocales class is a
>>> bit of a puzzle. Maybe it's OK to leave the suppression for that entire
>>> class.
>>>
>>> s'marks
>>>
>>>
>>> On 12/2/11 2:24 PM, Masayoshi Okutsu wrote:
>>>> Looks good to me.
>>>>
>>>> Thanks,
>>>> Masayoshi
>>>>
>>>> On 2011/12/02 10:56, Naoto Sato wrote:
>>>>> Hello,
>>>>>
>>>>> Could you please review these two changesets for the WCD? One is for
>>>>> classes
>>>>> that belongs to input method framework:
>>>>>
>>>>> http://cr.openjdk.java.net/~naoto/7117465/webrev.00/
>>>>>
>>>>> and the other is for (some of the) i18n related *.util.* classes:
>>>>>
>>>>> http://cr.openjdk.java.net/~naoto/7117469/webrev.00/
>>>>>
>>>>> Thanks!
>>>>> Naoto
>>