On Fri, 23 Dec 2022 23:32:45 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:

> I tried to clean up the documentation in the `java.awt.color` package and 
> specify how the `null` parameters are handled here and there. But it looks 
> like the `ICC_profile` class is too specific so I would like to discuss it 
> separately.
> 
> The ICC_Profile has these methods:
>  * `getInstance(int)`      - always throws specified 
> `IllegalArgumentException` on the wrong constant
>  * `getInstance(byte[])` - always throws specified `IllegalArgumentException` 
> on `null`
>  * `getInstance(String fileName)` - always throws unspecified 
> `NullPointerException` on `null`
>  * `getInstance(InputStream)`      - thrown unspecified 
> `NullPointerException` before 
> [JDK-8227816](https://github.com/openjdk/jdk/commit/af20c6b9c4a3a21c1e2c7f56721e1077e7d8f388)
>  and now throws specified but misleading `IOException: Stream closed` 
> exception on `null`
> 
> So we have 3 methods that can get null as a parameter and each throw a 
> different exception.
> 
> Note that all of the specs for the methods above have this part:
> 
>> @throws IllegalArgumentException If the stream does not contain valid ICC 
>> Profile data
> 
> So I have an idea to change `getInstance(String)` and 
> `getInstance(InputStream)` to throw `IllegalArgumentException`  even if I 
> personally prefer NPE to be thrown.
> 
> From another point of view the `ICC_Profile` profile has other methods:
>  * `write(String fileName)` - always throws unspecified 
> `NullPointerException` on `null`
>  * `write(OutputStream) `  - always throws unspecified `NullPointerException` 
> on `null`
> 
> I am not sure we can throw `IllegalArgumentException` from the `write` 
> methods, so the specification for `getInstance(String)` and 
> `getInstance(InputStream)` could be updated to throw NPE on null same as 
> related `write`.
> 
> Thoughts?

mrserb wrote 
=====
>From another point of view the ICC_Profile profile has other methods:

write(String fileName) - always throws unspecified NullPointerException on null
write(OutputStream)  - always throws unspecified NullPointerException on null
I am not sure we can throw IllegalArgumentException from the write methods, so 
the specification for getInstance(String) and getInstance(InputStream) could be 
updated to throw NPE on null same as related write.
========

Now .. when we take this into consideration,  I'm inclined to look at the 
get/write ones for fileName and outputstream as pairs and think it might be 
best to just specify the NPE for ALL cases and forgo the "unification" with the 
other cases.

Since

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

PR: https://git.openjdk.org/jdk/pull/11784

Reply via email to