On Sat, 9 Mar 2024 05:26:33 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/FixedCapacitySet.java
>> line 135:
>>
>>> 133: protected final void ensureNotFrozen() {
>>> 134: if (frozen) {
>>> 135: throw new UnsupportedOperationException();
>>
>> should we explain why? "the set is frozen" or something like that.
>
> I'm not against adding an explanation, but it's not needed as this is part of
> the `Set` contract (sets that can't be modified are specified to throw
> `UnsupportedOperationException`), for example for `add` in `Set`:
>
> * @throws UnsupportedOperationException if the {@code add} operation
> * is not supported by this set
>
> The concept of "freezing" the set is not visible to any users, it's just
> there to avoid having to make a final copy of the set. It's modifiable by the
> code that created it, but if they freeze it before exposing it to the outside
> world, then for all intents and purposes it is a read only set for anyone
> else.
But it is somewhat visible: **public** `void freeze();` (although it cannot be
invoked directly).
Edit: While I accept your reasoning that the functionality is effectively
hidden from the user, I still think that the exception should always explain
"why", if only to reduce the cognitive load. Think of all the countless man
hours saved collectively over the years. Yes, the answer can be found by
googling, stackoverflowing, and reading RTFM, but if the answer is right there
it is so much better. Wouldn't you agree?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1519862160
