fix TreeSet.ofAll with comparator and empty values (#2255)

nfekete · danieldietrich · commit 6179333eb308 · 2019-01-06T23:57:48.000+01:00
* test for TreeSet.removeAll case that removes all elements from the set * fix TreeSet.ofAll() discarding the comparator when creating empty set * removed unnecessary @SuppressWarnings * test for TreeSet.ofAll with explicit comparator and empty initial values
diff --git a/vavr/src/main/java/io/vavr/collection/TreeSet.java b/vavr/src/main/java/io/vavr/collection/TreeSet.java
@@ -192,7 +192,6 @@ public static <T extends Comparable<? super T>> TreeSet<T> fill(int n, Supplier<
         return fill(Comparators.naturalComparator(), n, s);
     }
 
-    @SuppressWarnings("unchecked")
     public static <T extends Comparable<? super T>> TreeSet<T> ofAll(Iterable<? extends T> values) {
         return ofAll(Comparators.naturalComparator(), values);
     }
@@ -201,10 +200,10 @@ public static <T extends Comparable<? super T>> TreeSet<T> ofAll(Iterable<? exte
     public static <T> TreeSet<T> ofAll(Comparator<? super T> comparator, Iterable<? extends T> values) {
         Objects.requireNonNull(comparator, "comparator is null");
         Objects.requireNonNull(values, "values is null");
-        if (values instanceof TreeSet && ((TreeSet) values).comparator() == comparator) {
+        if (values instanceof TreeSet && ((TreeSet<?>) values).comparator() == comparator) {
             return (TreeSet<T>) values;
         } else {
-            return values.iterator().hasNext() ? new TreeSet<>(RedBlackTree.ofAll(comparator, values)) : (TreeSet<T>) empty();
+            return values.iterator().hasNext() ? new TreeSet<>(RedBlackTree.ofAll(comparator, values)) : empty(comparator);
         }
     }
 
diff --git a/vavr/src/test/java/io/vavr/collection/TreeSetTest.java b/vavr/src/test/java/io/vavr/collection/TreeSetTest.java
@@ -236,6 +236,14 @@ public void shouldCollectNonEmpty() {
 
     // -- construct
 
+    @Test
+    public void shouldConstructEmptySetWithExplicitComparator() {
+        final TreeSet<Integer> ts = TreeSet.<Integer> of(Comparators.naturalComparator()
+            .reversed())
+            .addAll(Array.ofAll(1, 2, 3));
+        assertThat(ts.toArray()).isEqualTo(Array.of(3, 2, 1));
+    }
+
     @Test
     public void shouldConstructStreamFromEmptyJavaStream() {
         final TreeSet<Integer> actual = ofJavaStream(java.util.stream.Stream.<Integer>empty());
@@ -282,6 +290,17 @@ public void shouldKeepComparator() {
         final List<Integer> expected = List.of(3, 2, 1);
         assertThat(actual).isEqualTo(expected);
     }
+    
+    // -- removeAll
+    
+    @Test
+    public void shouldKeepComparatorOnRemoveAll() {
+        final TreeSet<Integer> ts = TreeSet.of(Comparators.naturalComparator()
+            .reversed(), 1, 2, 3)
+            .removeAll(Array.ofAll(1, 2, 3))
+            .addAll(Array.ofAll(4, 5, 6));
+        assertThat(ts.toArray()).isEqualTo(Array.of(6, 5, 4));
+    }
 
     // -- diff
 

Original file line number	Diff line number	Diff line change
`@@ -192,7 +192,6 @@ public static <T extends Comparable<? super T>> TreeSet<T> fill(int n, Supplier<`
`192`	`192`	`return fill(Comparators.naturalComparator(), n, s);`
`193`	`193`	`}`
`194`	`194`
`195`		`- @SuppressWarnings("unchecked")`
`196`	`195`	`public static <T extends Comparable<? super T>> TreeSet<T> ofAll(Iterable<? extends T> values) {`
`197`	`196`	`return ofAll(Comparators.naturalComparator(), values);`
`198`	`197`	`}`
`@@ -201,10 +200,10 @@ public static <T extends Comparable<? super T>> TreeSet<T> ofAll(Iterable<? exte`
`201`	`200`	`public static <T> TreeSet<T> ofAll(Comparator<? super T> comparator, Iterable<? extends T> values) {`
`202`	`201`	`Objects.requireNonNull(comparator, "comparator is null");`
`203`	`202`	`Objects.requireNonNull(values, "values is null");`
`204`		`- if (values instanceof TreeSet && ((TreeSet) values).comparator() == comparator) {`
	`203`	`+ if (values instanceof TreeSet && ((TreeSet<?>) values).comparator() == comparator) {`
`205`	`204`	`return (TreeSet<T>) values;`
`206`	`205`	`} else {`
`207`		`- return values.iterator().hasNext() ? new TreeSet<>(RedBlackTree.ofAll(comparator, values)) : (TreeSet<T>) empty();`
	`206`	`+ return values.iterator().hasNext() ? new TreeSet<>(RedBlackTree.ofAll(comparator, values)) : empty(comparator);`
`208`	`207`	`}`
`209`	`208`	`}`
`210`	`209`