On Thu, 23 Oct 2025 20:46:49 GMT, David Beaumont <[email protected]> wrote:
>> 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. Leave it alone, maybe it will ultimately show its value. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2457342522
