Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-25 Thread Michael Hixson
That would get me the functionality I want. It's the route Guava took, with ordering.onResultOf(function). If you do go this route, I suggest appliedTo as another possibility for the name. I do see this solution as a big improvement over what's there currently. It works great for most of the

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-07 Thread Ali Ebrahimi
Hi, for me this is more readable than current naming. ComparatorPeople cmp1 = compareWith(People::getFirstName). thenCompareWith(People::getLastName); Ali Ebrahimi On Thu, Mar 7, 2013 at 12:22 AM, Henry Jen henry@oracle.com wrote: On 03/06/2013 03:28

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-07 Thread Michael Hixson
On Wed, Mar 6, 2013 at 1:01 PM, Henry Jen henry@oracle.com wrote: On 03/06/2013 02:31 AM, Michael Hixson wrote: 1. Why disable the following code even though it is (theoretically) safe? ComparatorCharSequence a = comparing(CharSequence::length); ComparatorString b =

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-07 Thread Henry Jen
I think it all boiled down to re-use an existing Comparator to compare for another type. What about we added this to ComparatorT as a default method, default S ComparatorS apply(FunctionS, ? extends T keyExtractor) { Objects.requireNonNull(keyExtractor); return

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Paul Sandoz
Hi Henry, Minor thing. In Comparator: 194 * @param other the other comparator used when equals on this. 195 * @throws NullPointerException if the argument is null. 196 * @since 1.8 197 */ 198 default ComparatorT thenComparing(Comparator? super T other) { 199

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Michael Hixson
Hello, I'm not one of the people that you're looking at to review this, but I have to give this feedback anyway. I tried to give similar feedback on another mailing list and got no response (maybe I chose the wrong list). These changes have been bothering me. :) 1. Why disable the following

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Ali Ebrahimi
Hi, just one suggestion: rename comparing with compareWith 1) public static T, U extends Comparable? super U ComparatorT compareWith(Function? super T, ? extends U keyExtractor) { 2) default ComparatorT thenCompareWith(Comparator? super T other) Best Regards, Ali Ebrahimi On Wed, Mar 6,

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Henry Jen
On 03/06/2013 03:28 AM, Ali Ebrahimi wrote: Hi, just one suggestion: rename comparing with compareWith There was a round of discussion on naming. http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2012-November/000446.html I have my personal preference among proposals, but

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Henry Jen
On 03/06/2013 02:31 AM, Michael Hixson wrote: Hello, I'm not one of the people that you're looking at to review this, but I have to give this feedback anyway. I tried to give similar feedback on another mailing list and got no response (maybe I chose the wrong list). These changes have

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-02-06 Thread Erik Joelsson
The build change looks fine. The new build will not need any changes. /Erik On 2013-02-06 04:51, Henry Jen wrote: Hi, This is an update on previous reviewed version, there are two new method introduced for Comparators to convert a Comparator into a BinaryOperator and corresponding test cases.