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.
