[10] RFR 8134512 : provide Alpha-Numeric (logical) Comparator
Ivan Gerasimov
ivan.gerasimov at oracle.com
Fri Jul 28 09:29:00 UTC 2017
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.gerasimov at oracle.com
> <mailto:ivan.gerasimov at 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
>> <jbluettduncan at gmail.com <mailto:jbluettduncan at 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.gerasimov at oracle.com
>> <mailto:ivan.gerasimov at 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
More information about the core-libs-dev
mailing list