On Fri, 22 Nov 2024 19:07:34 GMT, Bradford Wetmore <wetm...@openjdk.org> wrote:
>> Sean Coffey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> enum Options and logic
>
> src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 101:
>
>> 99: }
>> 100: }
>> 101: isOn = (property.isEmpty() || property.equals("all"))
>
> What would you think about including `EMPTYALL` as an enum option, rather
> than a separate String property using separate processing checks? This would
> simplify the logic, and you no longer need to store `property` in the class
> as it's no longer needed. e.g.:
>
> isOn = activeComponents.contains(ComponentToken.EMPTYALL)
> || activeComponents.contains(ComponentToken.SSL));
>
> This would help in the `isOn()` section, where you just check for `EMPTYALL`
the current isOn logic is pretty tight now also:
public static boolean isOn(Opt option) {
return Opt.ALL.on || option.on;
}
> src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 154:
>
>> 152: }
>> 153:
>> 154: if (property.isEmpty() || property.equals("all")) {
>
> Then here this becomes:
>
> if (activeComponents.contains(EMPTYALL)) {
> return true;
> }
design changed since this comment - what we have now is probably as easy to
read:
public static boolean isOn(Opt option) {
return Opt.ALL.on || option.on;
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2352026377
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2352023953
