From e956dc0c43e0663bf483b367eee278b0633a52c5 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Thu, 6 Feb 2014 10:30:18 -0800 Subject: [PATCH] 8033590: java.util.Comparator::thenComparing has unnecessary type restriction Reviewed-by: psandoz --- .../share/classes/java/util/Comparator.java | 4 +- jdk/test/java/util/Comparator/TypeTest.java | 44 ++++++++++++++++++- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/jdk/src/share/classes/java/util/Comparator.java b/jdk/src/share/classes/java/util/Comparator.java index 6f9d1663e69..ecf8d64ea95 100644 --- a/jdk/src/share/classes/java/util/Comparator.java +++ b/jdk/src/share/classes/java/util/Comparator.java @@ -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 @@ public interface Comparator { * @see #thenComparing(Comparator) * @since 1.8 */ - default > Comparator thenComparing( + default Comparator thenComparing( Function keyExtractor, Comparator keyComparator) { diff --git a/jdk/test/java/util/Comparator/TypeTest.java b/jdk/test/java/util/Comparator/TypeTest.java index 69ae983197f..b060004d48e 100644 --- a/jdk/test/java/util/Comparator/TypeTest.java +++ b/jdk/test/java/util/Comparator/TypeTest.java @@ -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.TreeMap; 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 @@ public class TypeTest { } } + 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 void assertOrder(T o1, T o2, Comparator cmp) { if (cmp.compare(o1, o2) > 0) { System.out.println("Fail!!"); @@ -75,6 +96,8 @@ public class TypeTest { } } + // 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 @@ public class TypeTest { Map 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 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); + } }