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

[Joe Darcy]

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.