Hi Sergey,

Thanks for the review.

<<Can you please check other methods in this class since we update it anyway.
<<For example "setInstalledLookAndFeels" has @throws NPE but 
"installLookAndFeel" has not.

I have checked all the functions and I could not find any other API with wrong 
The "installLookAndFeel" API allows "null" as LookAndFeelInfo.  It is mentioned 
in the description that though "null" can be used, but it is not advisable to 
do. So null is allowed as LookAndFeelInfo. 
The "setInstalledLookAndFeels" throws an exception if the array itself is a 
null reference, but it also allow null LookAndFeelInfo. A array with only null 
references will not throw NPE

Following code does not throw a NPE.
        UIManager.installLookAndFeel(null, null);
UIManager.LookAndFeelInfo[]{null, null});

Following does throw a NPE and it is mentioned in the Javadoc
       UIManager. setInstalledLookAndFeels (null);

<<The test to prove that, is welcome as well.
I think I won't be able to write a test which fails before the fix and passes 
afterwards as it is just a documentation change. Can you please tell me what 
are the requirements from the test and I will try to write one?

Pankaj Bansal

-----Original Message-----
From: Sergey Bylokhov 
Sent: Wednesday, April 11, 2018 3:33 AM
To: Pankaj Bansal; swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11][JDK-8153532] RFR: Add @throws NPE javadoc to 
UIManager.setLookAndFeel(String) method description

Hi, Pankaj.
Can you please check other methods in this class since we update it anyway.
For example "setInstalledLookAndFeels" has @throws NPE but "installLookAndFeel" 
has not.

The test to prove that, is welcome as well.

On 10/04/2018 03:57, Pankaj Bansal wrote:
> Hi All,
> Please review a very simple fix for documentation change enhancement:
> JDK-8153532: https://bugs.openjdk.java.net/browse/JDK-8153532
> Webrev: http://cr.openjdk.java.net/~pbansal/8153532/webrev.00/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8201363
> UIManager.setLookAndFeel (String) throws NPE if the className is null. 
> But this is not updated in the API description. Made changes for the same.
> Regards,
> Pankaj Bansal

Best regards, Sergey.

Reply via email to