On Mon, 11 Mar 2024 14:54:45 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> 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?
I'd like someone else to weigh in on this. If this were a `Set` that eventually
would be publicly accessible somewhere (by normal FX users) I'd definitely not
want this. In this case however, it is (for now at least) only used internally,
so I don't care much if it has a message -- just be aware the users might see
this if this class ever **does** get exposed (like `BitSet` was).
I also don't understand your point about man hours saved and cognitive load.
The message is perfectly clear, you can't call that method, and the reason is
clearly documented, and in the context of collections, it means the set cannot
be mutated (by you). The creator of the set should be well aware that the set
is read only after being frozen. Any external class that it is being exposed to
does not need to be aware of how this is achieved (the method that you get it
from will clearly say it is an "immutable set" or some such). A message about
the set being frozen will just be confusing (What does frozen mean? How do I
unfreeze it? Who froze it? Why is this `Set` special from other unmodifiable
sets which don't mention this?)
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1519973968
