RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

Roger Riggs Roger.Riggs at Oracle.com
Wed Nov 25 21:22:40 UTC 2015


Hi Iris,

I see the JEP says JDK specific, but does that rule out putting the 
version API in a Java.* package?
It would support the values found in the java.version, etc properties.
Perhaps as an nested class of System or Runtime?

Version.java:

Line 213:  Seems a bit wasteful to reparse the string after the matcher 
has done its work;
    but does not use the groups for the version components.

229:  The IAE("Terminal...") exception may not be that easily 
understood; can it say concisely
    that the build and optional elements are missing

264: current() should cache the value from the parse; it is likely to be 
called more than once and parsing
     an version string is a relatively expensive.

334: the naming of one of the version elements as 'optional' may be 
confusing because optional is an adjective.
   Especially when the element is optional and the method named optional 
returns an Optional.
   Can the name be a cogent noun. how about 'info' for informational string

396:  I'm not sure why BigInteger is needed; other than perhaps because 
it has a constructor that takes a string.

433:  Optional has a .ifPresent(xxx) method that could be used to 
streamline the code.
       pre.ifPresent( v -> sb.append("-").append(v));


OracleVersion.java:   Can this be renamed more functionally to reflect 
that the 4'th component is a patch number.
    It might be useful to folks other than Oracle.

107: the constructor should be declared private since it is not needed 
outside the class.

I would have preferred the Tests to be written using TestNG.

Thanks, Roger



On 11/24/2015 8:54 PM, Iris Clark wrote:
> Hi.
>
> Please review the new classes jdk.Version and jdk.OracleVersion.  These are
> simple Java APIs to parse, validate, and compare version numbers.
>
>    Bug
>
>      8072379: Implement jdk.Version and jdk.OracleVersion
>      https://bugs.openjdk.java.net/browse/JDK-8072379
>
>    Webrev
>
>      http://cr.openjdk.java.net/~iris/verona/8072379/webrev.1/
>
>    JavaDoc
>
>      http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/Version.html
>      http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/OracleVersion.html
>
> The .java files are predominantly javadoc. The code is relatively
> straight-forward.
>
> jdk.Version is the representation of the JDK version string as described in
> JEP 223 ([0], 8061493).  The javadoc is largely taken from the description
> section in the JEP.  The API is described in the "API" section.
>
> jdk.OracleVersion extends jdk.Version and is the representation of the Oracle
> JDK version string.  Its only purpose is to interpret the fourth element of
> the version number as a patch release.
>
> There are some minor discrepancies between the implementation and the JEP.
> The JEP will need to be updated.  The most notable is the name of the package
> ("jdk" vs. the original "jdk.util").  The rename was recommended by Mark.
>
> Thanks,
> iris
>   
> [0] http://openjdk.java.net/jeps/223




More information about the core-libs-dev mailing list