On Thu, 20 Feb 2025 15:14:42 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Indeed, it *can be*, and I prefer this design. This way, the `isBuiltIn` >> field can't be changed after the object is constructed. I think it's a >> cleaner design. >> >> To achieve this, you have to add a constructor which accepts a boolean flag: >> `ICC_Profile(ProfileDeferralInfo pdi, boolean builtIn)`, and >> `ICC_ProfileRGB` and `ICC_ProfileGray` need to provide an additional >> constructor as well. >> >> Here's [a diff to your branch which achieves >> this](https://github.com/honkar-jdk/jdk/compare/BuiltInCheck...aivanov-jdk:jdk:harshitha/8346465-built-inprofiles). >> >> When [you compare the changeset that I propose to >> `jdk/master`](https://github.com/openjdk/jdk/compare/master...aivanov-jdk:jdk:harshitha/8346465-built-inprofiles), >> there are less differences, and `BuiltInProfile` remains an interface. Yet >> two more files `ICC_ProfileRGB` and `ICC_ProfileGray` are modified. > > After adding a constructor which accepts a boolean parameter, the > constructors `ICC_ProfileRGB(ProfileDeferralInfo pdi)` and > `ICC_ProfileGray(ProfileDeferralInfo pdi)` — these two can be removed then. > Can't this field be final and be set in constructor of the class? > > Indeed, it _can be_, and I prefer this design. This way, the `isBuiltIn` > > field can't be changed after the object is constructed. I think it's a > > cleaner design. A cleaner diff between your latest commit and my commit on top: https://github.com/openjdk/jdk/compare/8da82c10b5be3b92655abef95fd5be0c8b6eaa00..6729625f39c0dc47cd2588906b1793611996ca10 This link shouldn't become invalid after either of us removes the corresponding branches from our forks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1963780521
