Hi Jonathan!
On 7/24/17 3:42 AM, Jonathan Bluett-Duncan wrote:
You're welcome Ivan!
I'm just proofreading the new webrev, and a few more things have
occurred to me:
1. What happens if the comparators are given the elements {"1abc",
"2abc", "10abc"} to compare? Would they sort the elements as {"1abc",
"2abc", "10abc"} or as {"1abc", "10abc", "2abc"}?
What about the array {"x1yz", "x2yz", "x10yz"}?
I wonder if these two cases should be added to the test too and given
as additional examples in the javadocs.
These arrays would be sorted in the way you expect: i.e. {"1abc",
"2abc", "10abc"} and {"x1yz", "x2yz", "x10yz"}, respectively.
I agree it is worthwhile to choose a good descriptive example for the
javadoc, so it would make it clear for a reader what to expect from the
routine.
Probably, something similar to what they have in MSDN:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb759947(v=vs.85).aspx
2. The example arrays which you kindly added to the comparators'
javadoc have no whitespace at the start but one space between the last
element and the }, so I think there either should be no space at the
end or an extra space at the start.
Okay, I'll make it consistent.
3. Very sorry, error on my part: on the javadoc lines which now say
"then the corresponding numerical values are compared; otherwise", I
suggested to add a "then" at the start; re-reading it though, I
personally think it reads better without, but I would understand and
not press it further if you disagree and think that the "then" is a
useful addition.
Fixed, thanks!
I'll post the updated webrev later, once incorporate other suggestions.
With kind regards,
Ivan
Best regards,
Jonathan
On 24 Jul 2017 06:21, "Ivan Gerasimov" <ivan.gerasi...@oracle.com
<mailto:ivan.gerasi...@oracle.com>> wrote:
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/
<http://cr.openjdk.java.net/%7Eigerasim/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
--
With kind regards,
Ivan Gerasimov
