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