RFR: 8159055: ImageIcon.setImage can't handle null parameter [v22]

[Prasanta Sadhukhan]

On Mon, 28 Jul 2025 19:00:35 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   ArgType name change
>
> src/java.desktop/share/classes/javax/swing/ImageIcon.java line 64:
> 
>> 62:  * are preloaded using MediaTracker to monitor the loaded state
>> 63:  * of the image.
>> 64:  * If the image source parameter to a constructor or method is non-null,
> 
> Suggestion:
> 
>  * of the image.
>  *
>  * <p>
>  * If the image source parameter to a constructor or method is non-null,
> 
> I suggest putting the discussion on non-null parameters in its own paragraph for ease of reading the javadoc.

ok

> src/java.desktop/share/classes/javax/swing/ImageIcon.java line 206:
> 
>> 204:      * a string representation of the URL.
>> 205:      * @param location the URL for the image
>> 206:      * @throws {@code NullPointerException} if (@code null) URL is passed.
> 
> Suggestion:
> 
>      * @throws NullPointerException if a {@code null} URL is passed

ok

> src/java.desktop/share/classes/javax/swing/ImageIcon.java line 217:
> 
>> 215:      * @param image the image
>> 216:      * @param description a brief textual description of the image
>> 217:      * @throws {@code NullPointerException} if (@code null) Image is passed.
> 
> Suggestion:
> 
>      * @throws NullPointerException if a {@code null} image is passed
> 
> Here, _“image”_ shouldn't be capitalised, you mean it as the literal _“image”_ rather than a type.
> 
> I still prefer specifying the parameter name:
> Suggestion:
> 
>      * @throws NullPointerException if {@code image} is {@code null}

ok

> test/jdk/javax/swing/ImageIcon/ImageIconTest.java line 39:
> 
>> 37: import javax.swing.ImageIcon;
>> 38: 
>> 39: public class ImageIconTest {
> 
> Suggestion:
> 
> public class ImageIconNullParametersTest {
> 
> Be more specific?

it is not just testing for NullParameters

> test/jdk/javax/swing/ImageIcon/ImageIconTest.java line 52:
> 
>> 50:             fos.write(invalidData);
>> 51:         }
>> 52:         String fileName = (new File(System.getProperty("test.src", "."), imgName)).getName();
> 
> Why do we need jumping over the hoops?
> 
> You're creating an image, put it into the current directory that defaults to `scratch` directory in jtreg. The `scratch` directory is automatically cleaned by jtreg between tests, thus you won't even leave files in the source tree of JDK.

used file.deleteOnExit

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2239691809
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2239693016
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2239693481
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2239695863
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2239697154