On Mon, 11 Mar 2024 16:01:13 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> 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?)

It's a very minor point, John.  The exception by itself answers the question of 
"what" happened, but not "why".  All I suggest is to explain "why".

But again, it's just a minor suggestion.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1520017611

Reply via email to