8033590: java.util.Comparator::thenComparing has unnecessary type restriction
authorhenryjen
Thu, 06 Feb 2014 10:30:18 -0800
changeset 22944 a9e7afb60f4f
parent 22943 f35dda1a2873
child 22945 89dd803515d8
8033590: java.util.Comparator::thenComparing has unnecessary type restriction Reviewed-by: psandoz
jdk/src/share/classes/java/util/Comparator.java
jdk/test/java/util/Comparator/TypeTest.java
--- a/jdk/src/share/classes/java/util/Comparator.java	Fri Feb 07 09:04:17 2014 -0800
+++ b/jdk/src/share/classes/java/util/Comparator.java	Thu Feb 06 10:30:18 2014 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -235,7 +235,7 @@
      * @see #thenComparing(Comparator)
      * @since 1.8
      */
-    default <U extends Comparable<? super U>> Comparator<T> thenComparing(
+    default <U> Comparator<T> thenComparing(
             Function<? super T, ? extends U> keyExtractor,
             Comparator<? super U> keyComparator)
     {
--- a/jdk/test/java/util/Comparator/TypeTest.java	Fri Feb 07 09:04:17 2014 -0800
+++ b/jdk/test/java/util/Comparator/TypeTest.java	Thu Feb 06 10:30:18 2014 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,7 @@
 /**
  * @test
  * @summary Comparator API narrowing type test
+ * @bug 8009736 8033590
  * @run testng TypeTest
  */
 
@@ -33,6 +34,8 @@
 import java.util.Comparator;
 import org.testng.annotations.Test;
 
+import static org.testng.Assert.assertTrue;
+
 @Test(groups = "unit")
 public class TypeTest {
     static class Person {
@@ -66,6 +69,24 @@
         }
     }
 
+    static class Department {
+        Manager mgr;
+        String hr_code;
+
+        Department(Manager mgr, String hr) {
+            this.mgr = mgr;
+            this.hr_code = hr;
+        }
+
+        Manager getManager() {
+            return mgr;
+        }
+
+        String getHR() {
+            return hr_code;
+        }
+    }
+
     static <T> void assertOrder(T o1, T o2, Comparator<? super T> cmp) {
         if (cmp.compare(o1, o2) > 0) {
             System.out.println("Fail!!");
@@ -75,6 +96,8 @@
         }
     }
 
+    // Type tests just to make sure the code can compile and build
+    // Not necessarily need a meaningful result
     public void testOrder() {
         Manager m1 = new Manager("Manager", 2, 2000);
         Manager m2 = new Manager("Manager", 4, 1300);
@@ -93,4 +116,23 @@
         Map<String, Integer> map = new TreeMap<>();
         map.entrySet().stream().sorted(Map.Entry.comparingByKey(String.CASE_INSENSITIVE_ORDER));
     }
+
+    public void testJDK8033590() {
+        Manager a = new Manager("John Doe", 1234, 16);
+        Manager b = new Manager("Jane Roe", 2468, 16);
+        Department da = new Department(a, "X");
+        Department db = new Department(b, "X");
+
+        Comparator<Department> cmp = Comparator.comparing(Department::getHR)
+                .thenComparing(Department::getManager, Employee.C);
+        assertTrue(cmp.compare(da, db) < 0);
+
+        cmp = Comparator.comparing(Department::getHR)
+                .thenComparing(Department::getManager, Manager.C);
+        assertTrue(cmp.compare(da, db) == 0);
+
+        cmp = Comparator.comparing(Department::getHR)
+                .thenComparing(Department::getManager, Person.C);
+        assertTrue(cmp.compare(da, db) > 0);
+    }
 }