RFR: JDK-8346465 : Add a check in setData() to restrict the update of Built-In ICC_Profiles [v5]
Phil Race
prr at openjdk.org
Thu Feb 20 20:36:52 UTC 2025
On Thu, 20 Feb 2025 18:44:28 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:
>> The javadoc for `IllegalArgumentException` says, <q cite="https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/IllegalArgumentException.html">Thrown to indicate that a method has been passed <em>an illegal or inappropriate argument</em>.</q> (Emphasis mine.)
>>
>> Getting an `IllegalArgumentException` for a valid argument would be confusing.
>>
>> `IllegalStateException`, as you said, “signals that a method has been invoked at an <em>illegal or inappropriate</em> time”. It's exactly the state of the object: a *built-in* color profile cannot be modified.
>>
>> Although, there's no time where such a modification will be allowed, I still think `IllegalStateException` better conveys the meaning: changes to built-in profiles are *never* allowed.
>
>> 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".
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1964300315
More information about the client-libs-dev
mailing list