RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values
Joseph D. Darcy
joe.darcy at oracle.com
Wed Jun 15 23:31:16 UTC 2016
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