RFR: 8341551: Revisit jdk.internal.loader.URLClassPath.JarLoader after JEP 486 [v2]

[Jaikiran Pai]

On Wed, 4 Dec 2024 16:25:47 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   improve code comment
>
> src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 89:
> 
>> 87:         // JAR check is disabled by default and will be enabled only if the "disable JAR check"
>> 88:         // system property has been set to "false".
>> 89:         JAR_CHECKING_ENABLED = p != null && p.equals("false");
> 
> I throw `BooleanArithmeticOverflowException` :-)
> 
> I think this would be easier to follow if the static field name was aligned with the property name itself. It's easier if they have the same interpretation. 
> 
> So you could keep  `DISABLE_JAR_CHECKING` and just do 
> 
> `DISABLE_JAR_CHECKING = !"false".equals(p)`
> 
> and then just `if (DISABLE_JAR_CHECKING)` below.
> 
> If you don't like that, at least consider that the above can be simplified to `"false".equals(p)`

Hello Eirik, I have received similar inputs in other places when discussing this change. I myself had to think a few times on which naming to follow here to make it easier to understand the code. I'll consider your and other inputs and come back to this tomorrow afresh.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22545#discussion_r1869908682