RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values
Steve Drach
steve.drach at oracle.com
Thu Jun 16 21:44:14 UTC 2016
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.darcy at 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.drach at 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.
>
More information about the core-libs-dev
mailing list