Review request for IMF classes and Locale related classes

Stuart Marks stuart.marks at oracle.com
Fri Dec 2 17:41:13 PST 2011


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
>


More information about the jdk8-dev mailing list