<Swing Dev> [15] Review Request: 8237746 Fixing compiler warnings in src/demo/share/jfc
Marc Hoffmann
hoffmann at mountainminds.com
Wed Mar 11 20:22:32 UTC 2020
Hi Alexey,
I’m happy to add more small fixes at a later point in time. Please understand that I’m not a Swing expert. But I could help to make the code look cleaner and more like modern Java.
Also I think the example code is used in some regression tests, right? So I need to make sure not to break those. Is there documentation or any pointers how to run those tests?
Regards,
-marc
> On 11. Mar 2020, at 21:07, Alexey Ivanov <alexey.ivanov at oracle.com> wrote:
>
> 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