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

[Eirik Bjørsnøs]

On Wed, 4 Dec 2024 16:40:42 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> 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.
>
> We're stuck with the property name for compatibility, and the usage within the class is fairly limited.
> Generally, it is easier to understand the behavior having a feature that is enabled not a disable that is disabled.
> $0.02

Good :-)

If you end up keeping the `JAR_CHECKING_ENABLED` name, then introducing a local variable for the evaluation of the property may be useful. It should also fit better with the comment you introduced: 


String p = props.getProperty("sun.misc.URLClassPath.disableJarChecking");
// JAR check is disabled by default and will be enabled only if the "disable JAR check"
// system property has been set to "false".
 boolean jarCheckingDisabled = "false".equals(p);
JAR_CHECKING_ENABLED = !jarCheckingDisabled;


But in my opinion the above just makes it explicit that we flip a boolean here just to flip it back again on the use site.

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

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