<Swing Dev> [15] Review Request: 8237746 Fixing compiler warnings in src/demo/share/jfc
Alexey Ivanov
alexey.ivanov at oracle.com
Wed Mar 11 20:07:58 UTC 2020
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
More information about the swing-dev
mailing list