RFR: JDK-8346465 : Add a check in setData() to restrict the update of Built-In ICC_Profiles [v5]

[Alexey Ivanov]

On Thu, 20 Feb 2025 20:34:19 GMT, Phil Race <prr at openjdk.org> wrote:

>>> Although, there's no time where such a modification will be allowed
>> 
>> Exactly, it will throw exception every time a built-In profile is passed unlike IllegalStateException which is thrown only during an inappropriate time for instance when you are trying to add an element to the queue when it is already full. Plus setData() already throws IAE for other invalid argument cases.
>
> Although it is not a checked exception, the fact that IllegalArgumentException is already documented on this method is considered to outweigh any benefit of a new Exception type such as IllegalStateException. And IllegalArgumentException is not that far off from existing usage.
> Existing usage includes the case that "the specified data is not appropriate to be set in this profile".

> > Although, there's no time where such a modification will be allowed
> 
> Exactly, it will throw exception every time a built-In profile is passed unlike IllegalStateException which is thrown only during an inappropriate time for instance when you are trying to add an element to the queue when it is already full. Plus setData() already throws IAE for other invalid argument cases.

I see no contradiction here. Inappropriate time is a vague definition. For built-in object, it is always inappropriate time to call `setData`.

`IllegalArgumentException` implies that the argument is invalid, and if I change the argument, the call will succeed — but the call will never succeed for a built-in profile *because **the state of the object** doesn't allow it*.

This is why `IllegalStateException` is more appropriate.

> Although it is not a checked exception, the fact that IllegalArgumentException is already documented on this method is considered to outweigh any benefit of a new Exception type such as IllegalStateException.

I do not agree here. The exception type should convey the reason it's thrown, otherwise there wouldn't be so many types of exceptions.

And two `@throws` with the same type aren't allowed in javadoc, which means you have to add the new reason to throw `IllegalArgumentException` into the existing one.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1964365376