On Wed, 22 Oct 2025 19:33:44 GMT, Roger Riggs <[email protected]> wrote:
>> They are NOT opposites. It is not possible to use only one or these flags >> and continue to be logically correct. > > Hmm, the implementation of `isPreviewOnly()` is exactly > `!hasNormalVersion(flags)` Oh I see what you mean. The outer API is `isPreviewOnly()` because that matches the semantics of the flags it is used to create in the parent directory (ImageLocation flags). However, as stated in the comments, the private (completely encapsulated) flag values inside the class are best defined in terms of additive flags. This is so you can merge entries easily. If the inner flag was an "IS_PREVIEW_ONLY" flag, you'd have it *set* and later *cleared* when a new resource entry that *wasn't* a preview entry was processed. By making the flags "IS_XXX_VERSION", you just set them for each resource, completely independently, and then, when the references are merged, you just OR the flags together. I mean I could change the public API to be talking about "has normal version", but then the code that calculates the parent directory's flags will look less clear than it does now. Instead of "parent dir is preview only if all entries are preview only" it would be "parent directory is preview only if no child entries have normal versions". This is just an example of the internal working of an encapsulated class not matching its public API for technical reasons. But that's why you encapsulate things, so the inner working can do what they do best, and the outer API can be friendly to the callers. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2457273279
