From e78f920d23129115bb10889750b2be36fc3a019f Mon Sep 17 00:00:00 2001 From: Patryk Date: Sun, 23 Aug 2015 08:32:32 +0200 Subject: [PATCH 1/3] Add a failing test for Stream.groupBy.map If you execute a Stream.groupBy, followed by Stream.map, which tries to access the entry.{key,value} it will crash with an NPE. --- src/test/java/javaslang/collection/StreamTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/java/javaslang/collection/StreamTest.java b/src/test/java/javaslang/collection/StreamTest.java index daf49879e3..c98464a1bd 100644 --- a/src/test/java/javaslang/collection/StreamTest.java +++ b/src/test/java/javaslang/collection/StreamTest.java @@ -282,6 +282,14 @@ public void shouldStringifyNonNilEvaluatingFirstTail() { assertThat(stream.toString()).isEqualTo("Stream(1, 2, ?)"); } + @Test + public void shouldGroupByAndMapProperly() { + assertThat(Stream.of(1) + .groupBy(n -> n) + .map(longStreamEntry -> longStreamEntry.key) + .toList()).isEqualTo(List.of(1)); + } + // -- Serializable @Test(expected = InvalidObjectException.class) From 97c2259656d638f72a10266c488a2a8bd63ff7cd Mon Sep 17 00:00:00 2001 From: Patryk Date: Sun, 23 Aug 2015 14:49:38 +0200 Subject: [PATCH 2/3] Implement HashMap.map (tests included) As a "side effect" of running tests for `HashMap.map`, a problem with `HashSet.equals` was identified and fixed. --- .../java/javaslang/collection/HashMap.java | 4 +-- .../java/javaslang/collection/HashSet.java | 3 +- .../javaslang/collection/HashMapTest.java | 36 +++++++++++++++++++ .../javaslang/collection/HashSetTest.java | 8 +++++ .../java/javaslang/collection/StreamTest.java | 8 +++-- 5 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 src/test/java/javaslang/collection/HashMapTest.java diff --git a/src/main/java/javaslang/collection/HashMap.java b/src/main/java/javaslang/collection/HashMap.java index 67b9936cdf..004de42b52 100644 --- a/src/main/java/javaslang/collection/HashMap.java +++ b/src/main/java/javaslang/collection/HashMap.java @@ -258,10 +258,10 @@ public int length() { public Map map(BiFunction> mapper) { return null; } - @Override public Set map(Function, ? extends U> mapper) { - return null; + Objects.requireNonNull(mapper, "mapper is null"); + return foldLeft(HashSet.empty(), (acc, entry) -> acc.add(mapper.apply(entry))); } @Override diff --git a/src/main/java/javaslang/collection/HashSet.java b/src/main/java/javaslang/collection/HashSet.java index d7b170d07d..4f1b5589d3 100644 --- a/src/main/java/javaslang/collection/HashSet.java +++ b/src/main/java/javaslang/collection/HashSet.java @@ -481,11 +481,12 @@ public int hashCode() { } @Override + @SuppressWarnings("unchecked") public boolean equals(Object o) { if (o == this) { return true; } else if (o instanceof HashSet) { - final HashSet that = (HashSet) o; + final HashSet that = (HashSet) o; return this.iterator().equals(that.iterator()); } else { return false; diff --git a/src/test/java/javaslang/collection/HashMapTest.java b/src/test/java/javaslang/collection/HashMapTest.java new file mode 100644 index 0000000000..e8ef653f09 --- /dev/null +++ b/src/test/java/javaslang/collection/HashMapTest.java @@ -0,0 +1,36 @@ +/* / \____ _ ______ _____ / \____ ____ _____ + * / \__ \/ \ / \__ \ / __// \__ \ / \/ __ \ Javaslang + * _/ // _\ \ \/ / _\ \\_ \/ // _\ \ /\ \__/ / Copyright 2014-2015 Daniel Dietrich + * /___/ \_____/\____/\_____/____/\___\_____/_/ \_/____/ Licensed under the Apache License, Version 2.0 + */ +package javaslang.collection; + +import org.junit.Test; + +import static org.assertj.core.api.StrictAssertions.assertThat; + +public class HashMapTest { + + // -- map + + @Test + public void shouldMapEmpty() { + final Set expected = HashSet.empty(); + + final Set actual = HashMap.empty().map(entry -> entry.key); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void shouldMapNonEmpty() { + final Set expected = HashSet.of(1, 2); + final Set actual = + HashMap.of( + Map.Entry.of(1, "1"), + Map.Entry.of(2, "2")) + .map(entry -> entry.key); + + assertThat(actual).isEqualTo(expected); + } +} diff --git a/src/test/java/javaslang/collection/HashSetTest.java b/src/test/java/javaslang/collection/HashSetTest.java index 74a8c78b2d..d38a6d130f 100644 --- a/src/test/java/javaslang/collection/HashSetTest.java +++ b/src/test/java/javaslang/collection/HashSetTest.java @@ -6,11 +6,14 @@ package javaslang.collection; import org.assertj.core.api.*; +import org.junit.Test; import java.util.ArrayList; import java.util.NoSuchElementException; import java.util.stream.Collector; +import static org.junit.Assert.assertTrue; + public class HashSetTest extends AbstractTraversableTest { @Override @@ -294,6 +297,11 @@ public void shouldSlideNonNilBySize2() { // TODO } + @Test + public void shouldBeEqual() { + assertTrue(HashSet.of(1).equals(HashSet.of(1))); + } + boolean isThisLazyCollection() { return false; } diff --git a/src/test/java/javaslang/collection/StreamTest.java b/src/test/java/javaslang/collection/StreamTest.java index c98464a1bd..69bbd75f6f 100644 --- a/src/test/java/javaslang/collection/StreamTest.java +++ b/src/test/java/javaslang/collection/StreamTest.java @@ -8,13 +8,17 @@ import javaslang.Serializables; import javaslang.collection.Stream.Cons; import javaslang.collection.Stream.Nil; +import org.assertj.core.data.MapEntry; import org.junit.Test; import java.io.InvalidObjectException; +import java.util.AbstractMap; import java.util.ArrayList; +import java.util.function.Function; import java.util.stream.Collector; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.StrictAssertions.entry; public class StreamTest extends AbstractSeqTest { @@ -285,8 +289,8 @@ public void shouldStringifyNonNilEvaluatingFirstTail() { @Test public void shouldGroupByAndMapProperly() { assertThat(Stream.of(1) - .groupBy(n -> n) - .map(longStreamEntry -> longStreamEntry.key) + .groupBy(Function.identity()) + .map(entry -> entry.key) .toList()).isEqualTo(List.of(1)); } From f140aff5d81bb993affcdb3ad6d3f0319da55494 Mon Sep 17 00:00:00 2001 From: Patryk Date: Sun, 23 Aug 2015 23:14:41 +0200 Subject: [PATCH 3/3] Address review comment Thanks to @danieldietrich, HashSet.equals was further improved, eliminating the risk of ClassCastException. --- src/main/java/javaslang/collection/HashSet.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/javaslang/collection/HashSet.java b/src/main/java/javaslang/collection/HashSet.java index 4f1b5589d3..2206bb846d 100644 --- a/src/main/java/javaslang/collection/HashSet.java +++ b/src/main/java/javaslang/collection/HashSet.java @@ -480,14 +480,14 @@ public int hashCode() { return hash.get(); } - @Override @SuppressWarnings("unchecked") + @Override public boolean equals(Object o) { if (o == this) { return true; } else if (o instanceof HashSet) { - final HashSet that = (HashSet) o; - return this.iterator().equals(that.iterator()); + final HashSet that = (HashSet) o; + return this.length() == that.length() && ((HashSet) this).containsAll(that); } else { return false; }