Thank you Jonathan for looking into that!
I agree with all your suggestions.
Here's the updated webrev with suggested modifications:
WEBREV: http://cr.openjdk.java.net/~igerasim/8134512/02/webrev/
With kind regards,
Ivan
On 7/23/17 3:56 AM, Jonathan Bluett-Duncan wrote:
Meant to reply to core-libs-dev as well; doing so now.
Jonathan
On 23 July 2017 at 11:50, Jonathan Bluett-Duncan
<jbluettdun...@gmail.com <mailto:jbluettdun...@gmail.com>> wrote:
Hi Ivan,
I'm not a reviewer per se, but I thought I'd chime in.
There's a few things with Comparator.java which I think could be
improved:
1. `comparingNumericallyLeadingZerosAhead()` is a confusing name
for me, as the "ahead" has no meaning in my mind; IMO the name
`comparingNumericallyLeadingZerosFirst()` better implies that
"0001" < "001" < "01".
2. It wasn't clear to me from the javadoc that the comparators
compare strings like "abc9" and "abc10" as "abc9" < "abc10",
so I think they should include more examples.
3. There's a typo in the javadocs for both methods: "represets"
--> "represents".
4. Where the javadocs say "follows the procedure", I think it'd
make more grammatical sense if they said "does the following".
5. The lines which say "at the boundary of a subsequence,
consisting of decimal digits, the" would flow better if they
said "at the boundary of a subsequence *consisting solely of
decimal digits*, then the". Note the removal of the comma
between "subsequence" and "consisting".
Hope this helps!
Jonathan
On 23 July 2017 at 05:36, Ivan Gerasimov
<ivan.gerasi...@oracle.com <mailto:ivan.gerasi...@oracle.com>> wrote:
Hello!
This is a gentle reminder.
The proposed comparator implementation would be particularly
useful when one will need to compare version strings.
Some popular file managers also use similar comparing
algorithm, as the results often look more natural to the human
eyes (e.g. File Explorer on Windows, Files on Ubuntu).
Now, as Java 10 is been worked on, to sort the list of Java
names correctly, this kind of comparator is needed:
Look: a list { ... "Java 8", "Java 9", "Java 10" } definitely
looks nicer than { "Java 1", "Java 10", "Java 2", ... } :-)
Would you please help review the proposal?
With kind regards,
Ivan
On 7/19/17 1:41 AM, Ivan Gerasimov wrote:
Hello!
It is a proposal to provide a String comparator, which
will pay attention to the numbers embedded into the
strings (should they present).
This proposal was initially discussed back in 2014 and
seemed to bring some interest from the community:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-December/030343.html
<http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-December/030343.html>
In the latest webrev two methods are added to the public API:
j.u.Comparator.comparingNumerically() and
j.u.Comparator.comparingNumericallyLeadingZerosAhead().
The regression test is extended to exercise this new
comparator.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8134512
<https://bugs.openjdk.java.net/browse/JDK-8134512>
WEBREV:
http://cr.openjdk.java.net/~igerasim/8134512/01/webrev/
<http://cr.openjdk.java.net/%7Eigerasim/8134512/01/webrev/>
Comments, suggestions are very welcome!
--
With kind regards,
Ivan Gerasimov
--
With kind regards,
Ivan Gerasimov
