On Thu, 20 Feb 2025 14:43:57 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> Harshitha Onkar has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   javadoc change
>
> src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 116:
> 
>> 114:      * BuiltInProfile.
>> 115:      */
>> 116:     private boolean isBuiltIn = false;
> 
> 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.

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.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23606#discussion_r1963761853

Reply via email to