On Thu, 20 Feb 2025 20:34:19 GMT, Phil Race <p...@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
