Improved some methods in CharSeq, Vector, TreeSet and HashSet classes… #2208
Conversation
… using Collections.isEmpty(Iterable) method (vavr-io#1958)
There was a problem hiding this comment.
Hi Nataliia, many thanks for you PR :)
I think we should revert two methods, please see my comments.
@ruslansennov is Collections.isEmpty(...) GWT compatible? It internally uses instanceof but I think it is supported.
| @@ -494,11 +494,10 @@ private HashSet(HashArrayMappedTrie<T, T> tree) { | |||
| final HashSet<T> set = (HashSet<T>) elements; | |||
| return set; | |||
| } | |||
| final HashArrayMappedTrie<T, T> that = addAll(tree, elements); | |||
| if (that.size() == tree.size()) { | |||
| if (Collections.isEmpty(elements) || this.equals(elements)){ | |||
There was a problem hiding this comment.
The new way is only better when this HashSet and the elements are equal but it adds an overhead for all other cases. Please restore the previous version.
| @@ -532,17 +532,16 @@ | |||
| @Override | |||
| public TreeSet<T> addAll(Iterable<? extends T> elements) { | |||
| Objects.requireNonNull(elements, "elements is null"); | |||
| if (Collections.isEmpty(elements) || this.equals(elements)){ | |||
There was a problem hiding this comment.
A TreeSet is a SortedSet. We can't check for equality here because insertion depends on the underlying Comparator. Please restore the previous version.
|
🤔 Interestingly the CI tests failed both Java 8 and Java 9 for two Euler tests... Maybe the upcoming changes will heal them again. |
|
Thanks for comments! I'll revert those two methods. However it won't heal the problem with test (it is in Vector.ofAll() method). I have an idea, how to fix it. |
|
Thx! |
|
Sorry, I posted a non-relevant link :( Here we use But all is OK, this test works fine, I checked it package io.vavr.collection;
import com.google.gwt.junit.client.GWTTestCase;
public class CollTestGwt extends GWTTestCase {
public void testCollectionsIsEmpty() {
List<Integer> list = List.of(42);
assertFalse(Collections.isEmpty(list));
}
@Override
public String getModuleName() {
return "TestModule";
}
} |
Codecov Report
@@ Coverage Diff @@
## master #2208 +/- ##
============================================
+ Coverage 97.15% 97.16% +<.01%
- Complexity 5270 5273 +3
============================================
Files 93 93
Lines 12145 12155 +10
Branches 1593 1595 +2
============================================
+ Hits 11800 11810 +10
Misses 191 191
Partials 154 154
Continue to review full report at Codecov.
|
|
Thanks for the changes and sorry for the delay!! |
… using Collections.isEmpty(Iterable) method (#1958)