[OpenJDK 2D-Dev] JDK 9 RFR of JDK-8042864 : Fix raw and unchecked warnings in javax.print

[Phil Race]

OK now. I wouldn't necessarily agree the generification was poor, and it 
was a choice
made by someone else ..

-phil.

On 5/19/2014 12:49 PM, Joe Darcy wrote:
> Revised webev at
>
>     http://cr.openjdk.java.net/~darcy/8042864.2
>
> Comments below.
>
> On 05/19/2014 12:14 PM, Phil Race wrote:
>> On 5/19/2014 11:54 AM, Joe Darcy wrote:
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~darcy/8042864.0/src/share/classes/javax/print/attribute/standard/DialogTypeSelection.java.sdiff.html 
>>>>
>>>>
>>>> < 113 public final Class getCategory() {
>>>>
>>>> > 113 public final Class<DialogTypeSelection> getCategory() {
>>>>
>>>>
>>>> getCategory() is a super-class method ..
>>>>
>>>> So this seems wrong to me and I think|it should look like this ..|
>>>>
>>>> http://docs.oracle.com/javase/8/docs/api/javax/print/attribute/Attribute.html 
>>>>
>>>> |Class 
>>>> <http://docs.oracle.com/javase/8/docs/api/java/lang/Class.html><? 
>>>> extends Attribute 
>>>> <http://docs.oracle.com/javase/8/docs/api/javax/print/attribute/Attribute.html>>| 
>>>> |getCategory 
>>>> <http://docs.oracle.com/javase/8/docs/api/javax/print/attribute/Attribute.html#getCategory-->()
>>>>
>>>>
>>>> |
>>>
>>> Part of the generics language feature was covariant overrides, that 
>>> is, when a subtype overrides a methods from a supertype, the return 
>>> type of the subtype method can be a more-specific type.
>>>
>>> That is the case here; DialogTypeSelection is a subtype of "? 
>>> extends Attribute" so the getCategory method in DialogTypeSelection 
>>> is a covariant override of the superclass method. Moreover, since 
>>> the superclass uses the "? extends Attribute" wildcard, the 
>>> intention is for subclasses to override this method when possible.
>>>
>>
>>
>>
>> 1. FYI it appears I reviewed the .0 version since the reference to 
>> the .1 was so far down I didn't see it
>>
>> 2. I hadn't noticed until now that Henry also mentioned the 
>> getCategory() API ..
>>
>> 3. The 64 (!) other peer subclasses all use Class<? extends Attribute>.
>>    Yes, it will be a DialogTypeSelection but the primary use case is 
>> to be a member of a
>>   collection of Attribute so a more specific  API type isn't very 
>> useful at all .. and it
>>  will just stick out like a sore thumb.
>>
>>
>
> It is unfortunate that the other peer subclasses were suboptimally 
> generified to not use a covariant return when this API was initially 
> generified. From the sampling of methods I looked all, the methods in 
> question are final and state something like "This method must return 
> this specific Class object for the Foo class." Given that 
> specification, the typing of the getCategory method should reflect the 
> required behavior of the method and return a Class<Foo> rather than a 
> Class<? extends Attribute>.
>
> However, to be consistent with the existing poor generification, in 
> the .2 version of the webrev above I have changed the return type of 
> the method to the less informative Class<? extends Attribute>.
>
> -Joe
>
> PS I compile the classes before I send them out for a code review so 
> if there is a type error that would break the build, that is caught 
> beforehand.