On Fri, 20 Jan 2023 20:20:32 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? > > Sergey Bylokhov has updated the pull request incrementally with two > additional commits since the last revision: > > - Unify pairs for filenames and streams > - Revert "8299333: Unify used exceptions by all variants of > ICC_Profile.getInstance(null)" > > This reverts commit db5cfdc280c00e38949469f9bc6fcb3d06a12716. So there are 3 TCK tests for the behaviour of getInstance(InputStream) which expect the specified IOException or an IAE in the event the arg is null. It will fail if we throw NPE (ie these 3 tests fail on all platforms with your change) Interestingly at one point at least one of these expected NPE which probably got fixed because it wasn't the specified behaviour. I expect it'll make JCK very happy to be validated that NPE was better :-) ------------- PR: https://git.openjdk.org/jdk/pull/11784
