Hi Henry, If you don't mind could you hold off on committing this one until the following have gone through to tl:
http://cr.openjdk.java.net/~psandoz/tl/JDK-8016251-spinedBuffer-splitr/webrev/ http://cr.openjdk.java.net/~psandoz/tl/JDK-8016308-Node/webrev/ http://cr.openjdk.java.net/~psandoz/tl/JDK-8016324-pipelines/webrev/ http://cr.openjdk.java.net/~psandoz/tl/JDK-8016455-stream-tests/webrev/ as there is a delicate balance here to group the code into meaningful units for review and it's awkward to shuffle/update things. I rebased your patch off my patch queue, only one conflict required fixing. I can send you that queue in another email. Is that OK for you? -- Comparator.reverseOrder + * <p>The returned comparator is serializable. Try to compare null with + * returned comparator will throw {@link NullPointerException}. + * Typo "Try to compare" (and to . Do you mean: The compare method of the returned comparator will throw a {@link NullPointerException} if a {@code null} value is passed as the second parameter. ? Perhaps add a "@See Collections#reverseOrder" and vice versa on that method. Similar issue for Comparator.naturalOrder but for null passed as the first parameter. Comparators.comparing. Rather than "For example" you can use @apiNote .e.g * @apiNote * To obtain a .... The ToIntFunction accepting method has an example where as the long and double methods to not, perhaps just remove it. Comparators.NullComparator The "real" field can never be null: + NullComparator(int sign, Comparator<? super T> real) { + this.sign = sign; + this.real = (Comparator<T>) Objects.requireNonNull(real); + } but you are checking for null in the compare method + @Override + public int compare(T a, T b) { + if (a == null) { + return (b == null) ? 0 : sign; + } else if (b == null) { + return -sign; + } else { + return (real == null) ? 0 : real.compare(a, b); // <---- null check + } + } Map.comparingByKey/Value(Comparator<? super K/V> cmp) You don't mention "Note that a null key/value..." test/java/util/Comparator/BasicTest.java test/java/util/Comparator/TypeTest.java Has the wrong license header: + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. should be: + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Thanks, Paul. On Jun 11, 2013, at 11:04 PM, Henry Jen <henry....@oracle.com> wrote: > Hi, > > Please review http://cr.openjdk.java.net/~henryjen/ccc/8009736.2/webrev/ > > Highlight of changes, > > - Comparators class is now only package-private implementations. The > public static methods have been move to other arguably more appropriate > places, mostly in Comparator. > > - Comparator.reverseOrder() is renamed to Comparator.reversed() > > - nullsFirst(Comparator) and nullsLast(Comparator) are introduced to > wrap up a comparator to be null-friendly. > > To see the API changes, found the specdiff at > http://cr.openjdk.java.net/~henryjen/ccc/8009736.2/specdiff/overview-summary.html > > Cheers, > Henry
