Another comment below...
On 11/27/2015 6:36 AM, Magnus Ihse Bursie wrote:
On 2015-11-25 02:54, 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/
Hi Iris,
I thought the end agreement was that the + should always be present
even if build was empty, if opt was present but not pre. That is,
"9-foo" should unambigiously parse as vnum=9 and pre="foo", while
"9-+foo" should umambigously parse as vnum=9 and opt="foo". The
javadoc does not seem to reflect this.
I've also had to read and re-read the regexp and parsing logic in the
constructor to convince myself that this (most likely) will be
correctly handled. Perhaps a few comments around this would be helpful?
The comparison of two version strings which differs only in "pre" does
not adhere to the principle of astonishment.
The documentation states: "Pre-release identifiers are compared
numerically when they consist only of digits, and lexiographically
otherwise. Numeric identifiers are considered to be less than
non-numeric identifiers." But consider the following version strings:
9-01
9-01a
9-02
9-003b
That sequence would be ordered like this, which I find highly surprising.
9-01
9-02
9-003b
9-01a
I'm also surprised that equals() does not produce the same result as
compareTo(). If opt is to be ignored, surely it should be so in
equals() as well?
I'm one of the maintainers of BigDecimal, one of the few Comparable
classes in the JDK where compareTo is "inconsistent with equals" (see
"Effective Java, 2nd edition", Item 12 - Considering implementing
Comparable).
It is consistently surprising to users if compareTo is inconsistent with
equals ;-) Therefore, if at all possible it is preferable to have
compareTo be *consistent* rather than *inconsistent* with equals;
although there are cases where the inconsistency is necessary or at
least defensible.
I suggest providing multiple compareFoo methods for version that did or
did not include certain components, some of these could be consistent
with equals and others not.
HTH,
-Joe
