Hi Petr,
On 07/01/2014 01:17 AM, Petr Pchelko wrote:
Hello, Joe.
It's hard to understand what to review and what not to review. Could you please
generate a separate webrev for the changes that needs to be reviewed next time?
I've looked only to the javax.swing package, so I'm not sure I've reviewed
everything that needed to be reviewed.
1. JComboBox: 1983 - no need to suppress warning
Unnecessary suppression removed.
2. JComponent: 4121 - the change is incorrect. Value can be not an instance of
Set. It depends on the property name. See line 4124 where it's cast to Boolean
Noted; I've changed this to have a declaration of strokeSet in each of
the if block where the unchecked cast used to occur.
(Is is correct that this code is using "==" rather than ".equals" to
compare strings? If .equals were correct, I would change this method to
use strings in switch.)
3. SpinnerNumberModel: 90 - There's a typo in the comment with braces. Also, could you
please clarify what do you mean under "awkward to use"?
Typo fixed.
The actual Comparable mins and max people are likely to pass in are
values of type java.lang.Double, java.lang.Integer, etc. Such types are
correctly declared as Comparable to themselves:
public final class Double extends Number implements
Comparable<Double> {...}
public final class Integer extends Number implements
Comparable<Integer> {...}
This means that in the class for Double, there are two compareTo
methods, one that takes a Double argument and one that takes an Object
argument; the latter is the bridge method for the former. Of note is
that there is *not* a compareTo method taking a Number as an argument.
The Number type is a bit misnamed as an abstraction; it just means
"convertible to primitive." Therefore, two arbitrary Number types are
not mutually Comparable so it is sensible that Double doesn't have a
compareTo bridge method for Number.
If the minimum and maximum values were declared as Comparable<? extends
Number> with the aim of comparing two Number instances with them, in
practice that would mean calling a compareTo(Number) method which
doesn't exist. In that sense, it would be awkward to use Comparable<?
extends Number> ;-)
(This degree of explanation struck me as overkill for a comment, but I
can work on adding something along these lines if you think it would be
helpful to future readers of the code.)
The changes in javax.swing.tree and javax.swing.undo are new in this
request.
Thanks,
-Joe
With best regards. Petr.
On 30 июня 2014 г., at 20:20, Joe Darcy <joe.da...@oracle.com> wrote:
Hello,
Please review the final batch of changes to resolve all the unchecked and
rawtypes warnings in swing:
JDK-8043550 : Fix raw and unchecked lint warnings in javax.swing.*
http://cr.openjdk.java.net/~darcy/8043550.0/
For completeness, this webrev also includes the in-progress changes for
8043548: Fix raw and unchecked lint warnings in javax.swing.plaf.*
8042849: Fix raw and unchecked warnings in com.sun.java.swing
For 8043548, there is still some review work going on to sort out some of the
details; 8042849 has been approved, but not pushed yet as it could conceivably
need updating based on other changes in swing.
It was not immediately clear how javax.swing.tree.TreeNode.children(), which
returns a raw Enumeration, should be generified. I changed it to
Enumeration<TreeNode> children();
and that seems to work fine. Something like
Enumeration<? extends TreeNode>
with a covariant override would save a few casts in a private implementation,
but generally doesn't seem to be a good trade-off since many normal clients
could be inconvenienced dealing with the wildcard.
How to best generify the field javax.swing.KeyboardManager.containerMap was not
immediately clear either. The documentation implies the nested Hashtable could be
something like a Hashtable<KeyStroke, Object>
68 /**
69 * maps top-level containers to a sub-hashtable full of keystrokes
70 */
71 Hashtable<Container, Hashtable<Object, Object>> containerMap = new
Hashtable<>();
but that doesn't work out throughout the entire file since there is code that add
entries using a Class object as the key. In the end, I just used the not-very-precise
Hashtable<Object, Object>.
Thanks,
-Joe
