Finished HAMT.hashCode, optimized Vector.equals, fixed LinkedHashSet.…

…scanRight
vavr-io · Mar 30, 2016 · b4c8c56 · b4c8c56
1 parent ac6efc6
commit b4c8c56
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 17 deletions.
diff --git a/javaslang/src/main/java/javaslang/collection/HashArrayMappedTrie.java b/javaslang/src/main/java/javaslang/collection/HashArrayMappedTrie.java
@@ -143,27 +143,14 @@ public final boolean equals(Object o) {
                 return true;
             } else if (o instanceof HashArrayMappedTrie) {
                 final HashArrayMappedTrie<Object, ?> that = (HashArrayMappedTrie<Object, ?>) o;
-                if (this.size() != that.size()) {
-                    return false;
-                } else {
-                    for (Tuple2<K, V> entry : this) {
-                        Option<?> value = that.get(entry._1);
-                        if (value.isEmpty() || !Objects.equals(entry._2, value.get())) {
-                            return false;
-                        }
-                    }
-                    return true;
-                }
+                return (this.size() == that.size()) && Collections.equals(this, that);
             } else {
                 return false;
             }
         }
 
         @Override
-        public final int hashCode() {
-            // TODO: better hash algorithm to reduce collisions (e.g. (0 -> 1, 1 -> 2) and (0 -> 2, 1 -> 3) collide)
-            return iterator().foldLeft(0, (hash, entry) -> hash ^ entry.hashCode());
-        }
+        public abstract int hashCode();
 
         @Override
         public final String toString() {
@@ -216,6 +203,11 @@ public Iterator<Tuple2<K, V>> iterator() {
             return Iterator.empty();
         }
 
+        @Override
+        public int hashCode() {
+            return 0;
+        }
+
         /**
          * Instance control for object serialization.
          *
@@ -314,6 +306,11 @@ public Iterator<Tuple2<K, V>> iterator() {
             return Iterator.of(tuple);
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(hash, value);
+        }
+
         @Override
         int hash() {
             return hash;
@@ -442,6 +439,11 @@ public Tuple2<K, V> getNext() {
             };
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(hash, value, tail);
+        }
+
         @Override
         int hash() {
             return hash;
@@ -562,6 +564,11 @@ public int size() {
         public Iterator<Tuple2<K, V>> iterator() {
             return Iterator.concat(Array.wrap(subNodes));
         }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(subNodes);
+        }
     }
 
     /**
@@ -642,5 +649,10 @@ public int size() {
         public Iterator<Tuple2<K, V>> iterator() {
             return Iterator.concat(Array.wrap(subNodes));
         }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(subNodes);
+        }
     }
 }
diff --git a/javaslang/src/main/java/javaslang/collection/LinkedHashSet.java b/javaslang/src/main/java/javaslang/collection/LinkedHashSet.java
@@ -681,6 +681,7 @@ public LinkedHashSet<T> removeAll(Iterable<? extends T> elements) {
         return (that == map) ? this : new LinkedHashSet<>(that);
     }
 
+    // DEV-NOTE: replace does not preserve the order, the new element may already be in the set
     @Override
     public LinkedHashSet<T> replace(T currentElement, T newElement) {
         if (map.containsKey(currentElement)) {
@@ -725,7 +726,7 @@ public <U> LinkedHashSet<U> scanLeft(U zero, BiFunction<? super U, ? super T, ?
     @Override
     public <U> LinkedHashSet<U> scanRight(U zero, BiFunction<? super T, ? super U, ? extends U> operation) {
         Objects.requireNonNull(operation, "operation is null");
-        return Collections.scanRight(this, zero, operation, LinkedHashSet.empty(), LinkedHashSet::add, Function.identity());
+        return Collections.scanRight(this, zero, operation, List.empty(), List::prepend, LinkedHashSet::ofAll);
     }
 
     @Override

diff --git a/javaslang/src/main/java/javaslang/collection/Vector.java b/javaslang/src/main/java/javaslang/collection/Vector.java
@@ -1292,7 +1292,7 @@ public boolean equals(Object o) {
             return true;
         } else if (o instanceof Vector) {
             final Vector<?> that = (Vector<?>) o;
-            return Collections.equals(this, that);
+            return (this.size() == that.size()) && Collections.equals(this, that);
         } else {
             return false;
         }

diff --git a/javaslang/src/test/java/javaslang/collection/LinkedHashSetTest.java b/javaslang/src/test/java/javaslang/collection/LinkedHashSetTest.java
@@ -191,4 +191,12 @@ public void shouldTransform() {
         assertThat(transformed).isEqualTo("42");
     }
 
+    // -- replace(old, new)
+
+    @Override
+    @Test
+    public void shouldReplaceElementOfNonNilUsingCurrNewWhenOneOccurrenceExists() {
+        assertThat(of(0, 1, 2).replace(1, 3)).isEqualTo(of(0, 2, 3));
+    }
+
 }