Thank you to Marc for updating webrev and to Sergey for uploading it.
The changes look fine to me as I already stated.
I just wanted to share more comments:
On 22/02/2020 09:50, Sergey Bylokhov wrote:
Thank you, an updated version is upload:
http://cr.openjdk.java.net/~serb/8237746/webrev.01
On 2/17/20 11:55 am, Marc Hoffmann wrote:
Thanks Alexey for the detailed review! I attached a updated version.
The examples have many cleanup opportunities. I wanted to focus on
compiler warnings for now and keep the changeset minimal.
Yes, I agree. It's better to fix one problem at a time.
<SNIP>
*ButtonDemo.java*
64 Vector<Component> buttons = new Vector<>();
Shall it be JComponent?
Doesn’t because JPanel.add() returns Component:
buttons.add(p2.add(new JButton(getString("ButtonDemo.button1"))));
Should I introduce a local variable?
It could be another opportunity to contribute and clean up.
Vector can be replaced with ArrayList; and Component with
AbstractButton, then the type casts and instanceof checks in listeners
can be cleaned up too.
*ComboBoxDemo.java*
60 JComboBox<?> hairCB;
Why not JComboBox<String> ?
All createXXX methods use this type.
Then the cast below would be unnecessary:
282 String name = (String)
parts.get(hairCB.getSelectedItem());
This comes from the lookup in the parts Hashtable. Unfortunately it
has String an ImageIcon values.
Probably this could also be made cleaner then… Or maybe it's not worth it.
114 presetCB = (JComboBox<?>)
comboBoxPanel.add(createPresetComboBox());
To avoid cast, you can use two statements:
presetCB = createPresetComboBox();
comboBoxPanel.add(presetCB);
Done for all 4 occurrences.
*DirectionPanel.java*
97 AbstractButton b = e.nextElement();
98 if( b.getActionCommand().equals(selection) ) {
Indentation on line 97 seems incorrect, it should align to line 98,
shouldn't it?
Done (replace tab with spaces).
*SliderDemo.java*
167 @SuppressWarnings("unchecked")
168 Dictionary<Object, Object> labelTable =
s.getLabelTable();
Would using Dictionary<?, ?> suppress the warning automatically?
I mean that @SuppressWarning would become unnecessary.
Dictionary<?,?> does not allow put of specific types in the next
line. But fixed tabs in the same line.
I see, it comes from JSlider.get/setLabelTable which use the raw type
Dictionary.
So probably the API of JSlider itself can be updated to accept
Dictionary<Integer, ? extends JComponent>.
The generification of these two methods of JSlider was reverted to raw
types under https://bugs.openjdk.java.net/browse/JDK-8055254 because
SwingSet2 did not compile. So it should be a coordinated change to both
the API and the demo.
<SNIP>
--
Regards,
Alexey
