<Swing Dev> [15] Review Request: 8237746 Fixing compiler warnings in src/demo/share/jfc

Alexey Ivanov alexey.ivanov at oracle.com
Mon Feb 17 14:40:44 UTC 2020


Thank you, Marc, for your contribution.
And thank you to Sergey for creating the review.

*Font2DTest.java*
674         if ( selectedText == fp.USER_TEXT )
675           userTextDialog.setVisible(true);
676         else
677           userTextDialog.setVisible(false);

I'd put the braces around and indent the statements by 4 spaces.
However, it's the style used throughout the file: if there's only one 
statement, there are no braces and the statement is indented by 2 spaces 
rather than 4. Probably, to keep the code consistent, it's better to 
leave it as is.

797             else
798               fontInfoDialog.setVisible(false);


*FontPanel.java*
1248            if (valArray == null) {
1249                valArray = EnumSet.allOf(FMValues.class).toArray(new 
FMValues[0]);
1250            }
1259            if (valArray == null) {
1260                valArray = EnumSet.allOf(FMValues.class).toArray(new 
FMValues[0]);
1261            }
Can it be replaced with FMValues.values() as you did in Font2DTest.java 
lines 153, 156?

And below
1311                valArray = EnumSet.allOf(AAValues.class).toArray(new 
AAValues[0]);
1324                valArray = EnumSet.allOf(AAValues.class).toArray(new 
AAValues[0]);


*ButtonDemo.java*
64     Vector<Component> buttons = new Vector<>();
Shall it be JComponent?


*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());


114         presetCB = (JComboBox<?>) 
comboBoxPanel.add(createPresetComboBox());
To avoid cast, you can use two statements:
presetCB = createPresetComboBox();
comboBoxPanel.add(presetCB);


*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?


*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.


*SplitPaneDemo.java*
168 divSize.setText(Integer.valueOf(splitPane.getDividerSize()).toString());
can be simplified to
divSize.setText(Integer.toString(splitPane.getDividerSize()));
by using static method Integer.toString() method.


Shall the copyright year be updated in all the modified files?


On 23/01/2020 08:54, Marc Hoffmann wrote:
> Hi Sergey,
>
> thanks for sponsoring this patch!
>
> I successfully applied the webrev patch on current JDK head (57807:7bae17e00566). The build runs without warnings on the demo code :)
>
> But I noticed a minor glitch: I inserted a tab in src/demo/share/jfc/SwingSet2/DirectionPanel.java Line 97 where only spaces are used in the rest of the file. Probably this should be fixed before merge.
>
> Regards,
> -marc
>
>
>> On 23. Jan 2020, at 01:35, Sergey Bylokhov <Sergey.Bylokhov at oracle.com> wrote:
>>
>> Hello.
>> Please review the fix for compiler warnings in the demo/jfc in JDK 15.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8237746
>> Fix: http://cr.openjdk.java.net/~serb/8237746/webrev.00
>>
>> This fix contributed by the Marc Hoffmann:
>> https://mail.openjdk.java.net/pipermail/swing-dev/2019-October/009846.html
>>
>> Fixed warnings: raw types, deprecated APIs, deprecated Applet APIs.
>>
>> -- 
>> Best regards, Sergey.
>
-- 
Regards,
Alexey


More information about the swing-dev mailing list