This webrev uses methods instead of fields to return the base and runtime values used internally by JarFile. I’ve also optimized it a bit.
http://cr.openjdk.java.net/~sdrach/8150680/webrev.02/ <http://cr.openjdk.java.net/~sdrach/8150680/webrev.02/> > On Jun 15, 2016, at 4:31 PM, Joseph D. Darcy <joe.da...@oracle.com> wrote: > > Steve, > > In JarFile, please use methods not fields to return the new information. The > information in question is not constant across versions. Using methods > instead of fields avoid over-committing on a particular implementation, etc. > > Cheers, > > -Joe > > On 6/15/2016 3:49 PM, Steve Drach wrote: >> I’ve updated the webrev to address the issue of the constructor accepting >> values like Version.parse(“7.1”) >> >> http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/ >> <http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/> >> >>> On Jun 15, 2016, at 8:56 AM, Steve Drach <steve.dr...@oracle.com> wrote: >>> >>>>> Please review the following changeset: >>>>> >>>>> webrev: http://cr.openjdk.java.net/~sdrach/8150680/webrev.00/index.html >>>>> <http://cr.openjdk.java.net/~sdrach/8150680/webrev.00/index.html> >>>>> issue: https://bugs.openjdk.java.net/browse/JDK-8150680 >>>>> <https://bugs.openjdk.java.net/browse/JDK-8150680> >>>>> >>>>> The issue calls for reconsidering the JarFile.Release enum. A comment in >>>>> the bug report suggests replacing JarFile.Release with Runtime.Version, >>>>> and that’s what I did. Specifically I removed the enum, changed the >>>>> constructor to accept a Runtime.Version object instead of a >>>>> JarFile.Release object, updated all places in the JDK that invoked the >>>>> constructor and updated all tests. >>>>> >>>> Moving to Runtime.Version seems right but doesn't the javadoc for the >>>> constructor need to be updated to make it clear how it behavior when >>>> invoking with something like Version.parse("7.1") ? If I read the code >>>> correctly then this will be accepted and getVersion() will return 7.1. >>> Yes, it needs to be updated and it needs to be fixed. Thanks for finding >>> that. >>> >>>> Fields or methods is another discussion point for the base and runtime >>>> versions. >>> My thinking is, in this case fields and methods are equivalent, the method >>> not giving any more flexibility than a field. For example the method >>> JarFile.baseVersion will just return the value contained in the private >>> final static field BASE_VERSION. Or the public final static field >>> BASE_VERSION can be directly accessed. I see no advantage of a method >>> here. But I’m willing to be enlightened. >>> >>>> -Alan. >
