<Swing Dev> JDK 9 RFR of JDK-8054360: Refine generification of javax.swing
Robert Gibson
robbiexgibson at yahoo.com
Fri Aug 8 10:02:26 UTC 2014
On 8 Aug 2014, at 04:20, Joe Darcy <joe.darcy at oracle.com> wrote:
> Hello,
>
> Please review my changes for
>
> JDK-8054360: Refine generification of javax.swing h
> http://cr.openjdk.java.net/~darcy/8054360.3/
>
> Full patch below.
>
> This resolves many of the source incompatibility exemplars Jan Lahoda found in a corpus of programs using swing.
>
> A few comments on the changes.
>
> It seems many existing implementations of the TreeNode interface have a covariant override of the old raw
>
> Enumeration children();
>
> method which returns an enumeration of the specific TreeNode implementation type. The revised generification of
>
> Enumeration<? extends TreeNode> children();
>
> allows those existing implementations to still compile. This is a lower-impact way of allowing those types to still compile compared to trying to add a type variable to TreeNode.
>
> After some expert generics advice from Dan Smith, I but together a different generification of
>
> src/share/classes/javax/swing/table/DefaultTableModel.java
>
> which has better source compatibility. Quoting from the changes:
>
> 73 @SuppressWarnings("rawtypes")
> 74 protected Vector<Vector> dataVector;
> 75
> 76 /** The <code>Vector</code> of column identifiers. */
> 77 @SuppressWarnings("rawtypes")
> 78 protected Vector columnIdentifiers;
> 79 // Unfortunately, for greater source compatibility the inner-most
> 80 // Vector in the two fields above is being left raw. The Vector is
> 81 // read as well as written so using Vector<?> is not suitable and
> 82 // using Vector<Object> (without adding copying of input Vectors),
> 83 // would disallow existing code that used, say, a Vector<String>
> 84 // as an input parameter.
>
> The setter methods used for these fields are changes to have parameters of type Vector<?> and Vector<? extends Vector>, respectively.
>
> The type Vector<? extends Vector> is the most general type which captures the implicit constraint of the dataVector field: it is a Vector of other Vectors. (It would probably be possible to update dataVector to the somewhat more general Vector<List>, but that would require changes to the code in other methods of the class.)
>
> The changes in src/share/classes/sun/tools/jconsole/inspector/TableSorter.java change the code back to how it was before the swing generification changes went in based on the changes to DefaultTableModel.
>
> Once the exact generification is settled, I'll file the internal ccc paperwork.
>
> Early feedback on using this revised generification on swing code is welcome!
>
> Thanks,
>
> -Joe
>
> --- old/src/share/classes/javax/swing/JSlider.java 2014-08-07 16:53:58.000000000 -0700
> +++ new/src/share/classes/javax/swing/JSlider.java 2014-08-07 16:53:58.000000000 -0700
> @@ -138,7 +138,7 @@
> /**
> * {@code Dictionary} of what labels to draw at which values
> */
> - private Dictionary<Integer, JComponent> labelTable;
> + private Dictionary<Integer, ? extends JComponent> labelTable;
>
>
> /**
> @@ -773,7 +773,7 @@
> }
>
> // Check that there is a label with such image
> - Enumeration<JComponent> elements = labelTable.elements();
> + Enumeration<? extends JComponent> elements = labelTable.elements();
>
> while (elements.hasMoreElements()) {
> JComponent component = elements.nextElement();
> @@ -797,7 +797,7 @@
> * @return the <code>Dictionary</code> containing labels and
> * where to draw them
> */
> - public Dictionary<Integer, JComponent> getLabelTable() {
> + public Dictionary<Integer, ? extends JComponent> getLabelTable() {
> /*
> if ( labelTable == null && getMajorTickSpacing() > 0 ) {
> setLabelTable( createStandardLabels( getMajorTickSpacing() ) );
> @@ -830,8 +830,8 @@
> * attribute: visualUpdate true
> * description: Specifies what labels will be drawn for any given value.
> */
> - public void setLabelTable( Dictionary<Integer, JComponent> labels ) {
> - Dictionary<Integer, JComponent> oldTable = labelTable;
> + public void setLabelTable( Dictionary<Integer, ? extends JComponent> labels ) {
> + Dictionary<Integer, ? extends JComponent> oldTable = labelTable;
> labelTable = labels;
> updateLabelUIs();
> firePropertyChange("labelTable", oldTable, labelTable );
> @@ -852,7 +852,7 @@
> * @see JComponent#updateUI
> */
> protected void updateLabelUIs() {
> - Dictionary<Integer, JComponent> labelTable = getLabelTable();
> + Dictionary<Integer, ? extends JComponent> labelTable = getLabelTable();
>
> if (labelTable == null) {
> return;
> @@ -866,9 +866,9 @@
> }
>
> private void updateLabelSizes() {
> - Dictionary<Integer, JComponent> labelTable = getLabelTable();
> + Dictionary<Integer, ? extends JComponent> labelTable = getLabelTable();
> if (labelTable != null) {
> - Enumeration<JComponent> labels = labelTable.elements();
> + Enumeration<? extends JComponent> labels = labelTable.elements();
> while (labels.hasMoreElements()) {
> JComponent component = labels.nextElement();
> component.setSize(component.getPreferredSize());
> @@ -1017,7 +1017,7 @@
>
> SmartHashtable table = new SmartHashtable( increment, start );
>
> - Dictionary<Integer, JComponent> labelTable = getLabelTable();
> + Dictionary<Integer, ? extends JComponent> labelTable = getLabelTable();
>
> if (labelTable != null && (labelTable instanceof PropertyChangeListener)) {
> removePropertyChangeListener((PropertyChangeListener) labelTable);
> --- old/src/share/classes/javax/swing/JTable.java 2014-08-07 16:53:59.000000000 -0700
> +++ new/src/share/classes/javax/swing/JTable.java 2014-08-07 16:53:59.000000000 -0700
> @@ -669,7 +669,8 @@
> * @param rowData the data for the new table
> * @param columnNames names of each column
> */
> - public JTable(Vector<Vector<Object>> rowData, Vector<Object> columnNames) {
> + @SuppressWarnings("rawtypes")
> + public JTable(Vector<? extends Vector> rowData, Vector<?> columnNames) {
> this(new DefaultTableModel(rowData, columnNames));
> }
>
> --- old/src/share/classes/javax/swing/plaf/basic/BasicSliderUI.java 2014-08-07 16:54:00.000000000 -0700
> +++ new/src/share/classes/javax/swing/plaf/basic/BasicSliderUI.java 2014-08-07 16:54:00.000000000 -0700
> @@ -397,10 +397,10 @@
> protected boolean labelsHaveSameBaselines() {
> if (!checkedLabelBaselines) {
> checkedLabelBaselines = true;
> - Dictionary<?, JComponent> dictionary = slider.getLabelTable();
> + Dictionary<Integer, ? extends JComponent> dictionary = slider.getLabelTable();
> if (dictionary != null) {
> sameLabelBaselines = true;
> - Enumeration<JComponent> elements = dictionary.elements();
> + Enumeration<? extends JComponent> elements = dictionary.elements();
> int baseline = -1;
> while (elements.hasMoreElements()) {
> JComponent label = elements.nextElement();
> @@ -753,7 +753,7 @@
> }
>
> protected int getWidthOfWidestLabel() {
> - Dictionary<?, JComponent> dictionary = slider.getLabelTable();
> + Dictionary<?, ? extends JComponent> dictionary = slider.getLabelTable();
> int widest = 0;
> if ( dictionary != null ) {
> Enumeration<?> keys = dictionary.keys();
> @@ -766,7 +766,7 @@
> }
>
> protected int getHeightOfTallestLabel() {
> - Dictionary<?, JComponent> dictionary = slider.getLabelTable();
> + Dictionary<?, ? extends JComponent> dictionary = slider.getLabelTable();
> int tallest = 0;
> if ( dictionary != null ) {
> Enumeration<?> keys = dictionary.keys();
> @@ -871,7 +871,7 @@
> * @since 1.6
> */
> protected Integer getLowestValue() {
> - Dictionary<Integer, JComponent> dictionary = slider.getLabelTable();
> + Dictionary<Integer, ? extends JComponent> dictionary = slider.getLabelTable();
>
> if (dictionary == null) {
> return null;
> @@ -1121,7 +1121,7 @@
> public void paintLabels( Graphics g ) {
> Rectangle labelBounds = labelRect;
>
> - Dictionary<Integer, JComponent> dictionary = slider.getLabelTable();
> + Dictionary<Integer, ? extends JComponent> dictionary = slider.getLabelTable();
> if ( dictionary != null ) {
> Enumeration<Integer> keys = dictionary.keys();
> int minValue = slider.getMinimum();
> --- old/src/share/classes/javax/swing/plaf/synth/SynthSliderUI.java 2014-08-07 16:54:02.000000000 -0700
> +++ new/src/share/classes/javax/swing/plaf/synth/SynthSliderUI.java 2014-08-07 16:54:01.000000000 -0700
> @@ -392,7 +392,7 @@
> trackRect.x = insetCache.left;
> trackRect.width = contentRect.width;
>
> - Dictionary<Integer, JComponent> dictionary = slider.getLabelTable();
> + Dictionary<Integer, ? extends JComponent> dictionary = slider.getLabelTable();
> if (dictionary != null) {
> int minValue = slider.getMinimum();
> int maxValue = slider.getMaximum();
> --- old/src/share/classes/javax/swing/table/DefaultTableModel.java 2014-08-07 16:54:03.000000000 -0700
> +++ new/src/share/classes/javax/swing/table/DefaultTableModel.java 2014-08-07 16:54:02.000000000 -0700
> @@ -70,10 +70,18 @@
> * The <code>Vector</code> of <code>Vectors</code> of
> * <code>Object</code> values.
> */
> - protected Vector<Vector<Object>> dataVector;
> + @SuppressWarnings("rawtypes")
> + protected Vector<Vector> dataVector;
>
> /** The <code>Vector</code> of column identifiers. */
> - protected Vector<Object> columnIdentifiers;
> + @SuppressWarnings("rawtypes")
> + protected Vector columnIdentifiers;
> + // Unfortunately, for greater source compatibility the inner-most
> + // Vector in the two fields above is being left raw. The Vector is
> + // read as well as written so using Vector<?> is not suitable and
> + // using Vector<Object> (without adding copying of input Vectors),
> + // would disallow existing code that used, say, a Vector<String>
> + // as an input parameter.
>
> //
> // Constructors
> @@ -121,7 +129,7 @@
> * @see #setDataVector
> * @see #setValueAt
> */
> - public DefaultTableModel(Vector<Object> columnNames, int rowCount) {
> + public DefaultTableModel(Vector<?> columnNames, int rowCount) {
> setDataVector(newVector(rowCount), columnNames);
> }
>
> @@ -156,7 +164,8 @@
> * @see #getDataVector
> * @see #setDataVector
> */
> - public DefaultTableModel(Vector<Vector<Object>> data, Vector<Object> columnNames) {
> + @SuppressWarnings("rawtypes")
> + public DefaultTableModel(Vector<? extends Vector> data, Vector<?> columnNames) {
> setDataVector(data, columnNames);
> }
>
> @@ -191,7 +200,8 @@
> * @see #newRowsAdded
> * @see #setDataVector
> */
> - public Vector<Vector<Object>> getDataVector() {
> + @SuppressWarnings("rawtypes")
> + public Vector<Vector> getDataVector() {
> return dataVector;
> }
>
> @@ -219,9 +229,10 @@
> * @param columnIdentifiers the names of the columns
> * @see #getDataVector
> */
> - public void setDataVector(Vector<Vector<Object>> dataVector,
> - Vector<Object> columnIdentifiers) {
> - this.dataVector = nonNullVector(dataVector);
> + @SuppressWarnings({"rawtypes", "unchecked"})
> + public void setDataVector(Vector<? extends Vector> dataVector,
> + Vector<?> columnIdentifiers) {
> + this.dataVector = nonNullVector((Vector<Vector>)dataVector);
> this.columnIdentifiers = nonNullVector(columnIdentifiers);
> justifyRows(0, getRowCount());
> fireTableStructureChanged();
> @@ -267,7 +278,7 @@
> if (dataVector.elementAt(i) == null) {
> dataVector.setElementAt(new Vector<>(), i);
> }
> - ((Vector)dataVector.elementAt(i)).setSize(getColumnCount());
> + dataVector.elementAt(i).setSize(getColumnCount());
> }
> }
>
> @@ -350,7 +361,7 @@
> *
> * @param rowData optional data of the row being added
> */
> - public void addRow(Vector<Object> rowData) {
> + public void addRow(Vector<?> rowData) {
> insertRow(getRowCount(), rowData);
> }
>
> @@ -374,7 +385,7 @@
> * @param rowData optional data of the row being added
> * @exception ArrayIndexOutOfBoundsException if the row was invalid
> */
> - public void insertRow(int row, Vector<Object> rowData) {
> + public void insertRow(int row, Vector<?> rowData) {
> dataVector.insertElementAt(rowData, row);
> justifyRows(row, row+1);
> fireTableRowsInserted(row, row);
> @@ -484,7 +495,7 @@
> * to zero columns
> * @see #setNumRows
> */
> - public void setColumnIdentifiers(Vector<Object> columnIdentifiers) {
> + public void setColumnIdentifiers(Vector<?> columnIdentifiers) {
> setDataVector(dataVector, columnIdentifiers);
> }
>
> @@ -550,7 +561,8 @@
> * @param columnName the identifier of the column being added
> * @param columnData optional data of the column being added
> */
> - public void addColumn(Object columnName, Vector<Object> columnData) {
> + @SuppressWarnings("unchecked") // Adding element to raw columnIdentifiers
> + public void addColumn(Object columnName, Vector<?> columnData) {
> columnIdentifiers.addElement(columnName);
> if (columnData != null) {
> int columnSize = columnData.size();
> @@ -652,6 +664,7 @@
> * column was given
> */
> public Object getValueAt(int row, int column) {
> + @SuppressWarnings("unchecked")
> Vector<Object> rowVector = dataVector.elementAt(row);
> return rowVector.elementAt(column);
> }
> @@ -668,6 +681,7 @@
> * column was given
> */
> public void setValueAt(Object aValue, int row, int column) {
> + @SuppressWarnings("unchecked")
> Vector<Object> rowVector = dataVector.elementAt(row);
> rowVector.setElementAt(aValue, column);
> fireTableCellUpdated(row, column);
> --- old/src/share/classes/javax/swing/tree/DefaultMutableTreeNode.java 2014-08-07 16:54:04.000000000 -0700
> +++ new/src/share/classes/javax/swing/tree/DefaultMutableTreeNode.java 2014-08-07 16:54:04.000000000 -0700
> @@ -1308,7 +1308,7 @@
> }
>
> private final class PreorderEnumeration implements Enumeration<TreeNode> {
> - private final Stack<Enumeration<TreeNode>> stack = new Stack<>();
> + private final Stack<Enumeration<? extends TreeNode>> stack = new Stack<>();
>
> public PreorderEnumeration(TreeNode rootNode) {
> super();
> @@ -1322,10 +1322,9 @@
> }
>
> public TreeNode nextElement() {
> - Enumeration<TreeNode> enumer = stack.peek();
> + Enumeration<? extends TreeNode> enumer = stack.peek();
> TreeNode node = enumer.nextElement();
> - @SuppressWarnings("unchecked")
> - Enumeration<TreeNode> children = node.children();
> + Enumeration<? extends TreeNode> children = node.children();
>
> if (!enumer.hasMoreElements()) {
> stack.pop();
> @@ -1342,7 +1341,7 @@
>
> final class PostorderEnumeration implements Enumeration<TreeNode> {
> protected TreeNode root;
> - protected Enumeration<TreeNode> children;
> + protected Enumeration<? extends TreeNode> children;
> protected Enumeration<TreeNode> subtree;
>
> public PostorderEnumeration(TreeNode rootNode) {
> --- old/src/share/classes/javax/swing/tree/TreeNode.java 2014-08-07 16:54:05.000000000 -0700
> +++ new/src/share/classes/javax/swing/tree/TreeNode.java 2014-08-07 16:54:05.000000000 -0700
> @@ -99,5 +99,5 @@
> *
> * @return the children of the receiver as an {@code Enumeration}
> */
> - Enumeration<TreeNode> children();
> + Enumeration<? extends TreeNode> children();
> }
> --- old/src/share/classes/sun/tools/jconsole/inspector/TableSorter.java 2014-08-07 16:54:06.000000000 -0700
> +++ new/src/share/classes/sun/tools/jconsole/inspector/TableSorter.java 2014-08-07 16:54:06.000000000 -0700
> @@ -146,7 +146,7 @@
> // update row heights in XMBeanAttributes (required by expandable cells)
> if (attrs != null) {
> for (int i = 0; i < getRowCount(); i++) {
> - Vector<?> data = (Vector) dataVector.elementAt(i);
> + Vector<?> data = dataVector.elementAt(i);
> attrs.updateRowHeight(data.elementAt(1), i);
> }
> }
> @@ -217,17 +217,17 @@
> }
> }
>
> - private Vector<Object> getRow(int row) {
> + private Vector<?> getRow(int row) {
> return dataVector.elementAt(row);
> }
>
> @SuppressWarnings("unchecked")
> - private void setRow(Vector<Object> data, int row) {
> + private void setRow(Vector<?> data, int row) {
> dataVector.setElementAt(data,row);
> }
>
> private void swap(int i, int j, int column) {
> - Vector<Object> data = getRow(i);
> + Vector<?> data = getRow(i);
> setRow(getRow(j),i);
> setRow(data,j);
>
>
Hi Joe,
These changes look better for us. Note that we still have one incompatibility in a TreeNode stub implementation:
public Enumeration<?> getChildren()
{
throw new RuntimeException(“Not implemented”);
}
but this one is a hopeless case anyway as a pretty bad stab at approximating a bottom type, so we will end up changing it.
Regards,
Robert
More information about the swing-dev
mailing list