From e305cb20f84f1bcf04e62dde2b7e5bb0d5371adb Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Tue, 5 May 2020 10:34:09 +0200 Subject: [PATCH 01/15] Reproduce the problem --- .../java/io/vavr/issues/Issue2559Test.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 src/test/java/io/vavr/issues/Issue2559Test.java diff --git a/src/test/java/io/vavr/issues/Issue2559Test.java b/src/test/java/io/vavr/issues/Issue2559Test.java new file mode 100644 index 0000000000..74898cfa41 --- /dev/null +++ b/src/test/java/io/vavr/issues/Issue2559Test.java @@ -0,0 +1,86 @@ +package io.vavr.issues; + +import io.vavr.Tuple2; +import io.vavr.collection.HashSet; +import io.vavr.collection.Set; +import org.junit.Before; +import org.junit.Test; + +import java.util.Objects; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * partition method seems to not work correctly, comparing to scala + * https://github.com/vavr-io/vavr/issues/2559 + */ +public class Issue2559Test { + + private java.util.Map fruitsBeingEaten = new java.util.HashMap<>(); + + @Before + public void setUp() { + fruitsBeingEaten = new java.util.HashMap<>(); + } + + @Test + public void partitionShouldBeUnique() { + final Set fruitsToEat = HashSet.of("apple", "banana"); + final Tuple2, ? extends Set> partition = fruitsToEat.partition(this::biteAndCheck); + assertThat(partition._1).isEmpty(); + assertThat(partition._2).isEmpty(); + assertThat(fruitsBeingEaten) + .hasSize(2) + .containsEntry("apple", new Eat(2, "apple")) + .containsEntry("banana", new Eat(2, "banana")); + } + + private boolean biteAndCheck(String name) { + final Eat eat = fruitsBeingEaten.getOrDefault(name, Eat.prepare(name)).bite(); + fruitsBeingEaten.put(name, eat); + return eat.isEaten(); + } + + private static class Eat { + final int bites; + final String name; + + public static Eat prepare(String name) { + return new Eat(0, name); + } + + private Eat(int bites, String name) { + this.bites = bites; + this.name = name; + } + + public Eat bite() { + return new Eat(bites + 1, name); + } + + public boolean isEaten() { + return bites >= 2; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Eat)) return false; + Eat eat = (Eat) o; + return bites == eat.bites && Objects.equals(name, eat.name); + } + + @Override + public int hashCode() { + return Objects.hash(bites, name); + } + + @Override + public String toString() { + return "Eat{" + + "bites=" + bites + + ", name='" + name + '\'' + + '}'; + } + } +} From 1d2576625affa26790a106ab8acbfa60d41156d2 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Tue, 5 May 2020 12:08:33 +0200 Subject: [PATCH 02/15] Iterate once to create two iterators in partition --- src/main/java/io/vavr/collection/Iterator.java | 14 ++++++++++---- src/test/java/io/vavr/issues/Issue2559Test.java | 6 +++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/vavr/collection/Iterator.java b/src/main/java/io/vavr/collection/Iterator.java index 4e76922280..8111953479 100644 --- a/src/main/java/io/vavr/collection/Iterator.java +++ b/src/main/java/io/vavr/collection/Iterator.java @@ -1722,10 +1722,16 @@ default Tuple2, Iterator> partition(Predicate predicat if (!hasNext()) { return Tuple.of(empty(), empty()); } else { - final Stream that = Stream.ofAll(this); - final Iterator first = that.iterator().filter(predicate); - final Iterator second = that.iterator().filter(predicate.negate()); - return Tuple.of(first, second); + Stream first = Stream.empty(); + Stream second = Stream.empty(); + for (T t : this) { + if (predicate.test(t)) { + first = first.append(t); + } else { + second = second.append(t); + } + } + return Tuple.of(first.iterator(), second.iterator()); } } diff --git a/src/test/java/io/vavr/issues/Issue2559Test.java b/src/test/java/io/vavr/issues/Issue2559Test.java index 74898cfa41..8a11b2dbc1 100644 --- a/src/test/java/io/vavr/issues/Issue2559Test.java +++ b/src/test/java/io/vavr/issues/Issue2559Test.java @@ -28,11 +28,11 @@ public void partitionShouldBeUnique() { final Set fruitsToEat = HashSet.of("apple", "banana"); final Tuple2, ? extends Set> partition = fruitsToEat.partition(this::biteAndCheck); assertThat(partition._1).isEmpty(); - assertThat(partition._2).isEmpty(); + assertThat(partition._2).isEqualTo(HashSet.of("apple", "banana")); assertThat(fruitsBeingEaten) .hasSize(2) - .containsEntry("apple", new Eat(2, "apple")) - .containsEntry("banana", new Eat(2, "banana")); + .containsEntry("apple", new Eat(1, "apple")) + .containsEntry("banana", new Eat(1, "banana")); } private boolean biteAndCheck(String name) { From 7c52962699131f07cdb03e5b3e455766a07c3a70 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Tue, 5 May 2020 18:33:43 +0200 Subject: [PATCH 03/15] Avoid using io.vavr.collection.Stream --- src/main/java/io/vavr/collection/Iterator.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/vavr/collection/Iterator.java b/src/main/java/io/vavr/collection/Iterator.java index 8111953479..d8a7649435 100644 --- a/src/main/java/io/vavr/collection/Iterator.java +++ b/src/main/java/io/vavr/collection/Iterator.java @@ -1722,16 +1722,16 @@ default Tuple2, Iterator> partition(Predicate predicat if (!hasNext()) { return Tuple.of(empty(), empty()); } else { - Stream first = Stream.empty(); - Stream second = Stream.empty(); + final java.util.List first = new ArrayList<>(); + final java.util.List second = new ArrayList<>(); for (T t : this) { if (predicate.test(t)) { - first = first.append(t); + first.add(t); } else { - second = second.append(t); + second.add(t); } } - return Tuple.of(first.iterator(), second.iterator()); + return Tuple.of(ofAll(first), ofAll(second)); } } From 42117bf323ca26376972ead5a3e243414c701280 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Sat, 16 May 2020 12:02:01 +0200 Subject: [PATCH 04/15] Test behavior of `partition` on different classes --- .../io/vavr/collection/AbstractMapTest.java | 15 +++++++++++++++ .../io/vavr/collection/AbstractSetTest.java | 18 +++++++++++++++++- .../java/io/vavr/collection/ArrayTest.java | 15 +++++++++++++++ .../java/io/vavr/collection/CharSeqTest.java | 14 ++++++++++++++ src/test/java/io/vavr/collection/ListTest.java | 15 +++++++++++++++ .../java/io/vavr/collection/QueueTest.java | 15 +++++++++++++++ .../java/io/vavr/collection/StreamTest.java | 15 +++++++++++++++ .../java/io/vavr/collection/VectorTest.java | 16 ++++++++++++++++ 8 files changed, 122 insertions(+), 1 deletion(-) diff --git a/src/test/java/io/vavr/collection/AbstractMapTest.java b/src/test/java/io/vavr/collection/AbstractMapTest.java index 56b47ececd..4316e58a94 100644 --- a/src/test/java/io/vavr/collection/AbstractMapTest.java +++ b/src/test/java/io/vavr/collection/AbstractMapTest.java @@ -1461,6 +1461,21 @@ public void shouldReturnDefaultValue() { assertThat(map.getOrElse("3", "3")).isEqualTo("3"); } + // -- partition + + @Test + public void shouldPartitionInOneIteration() { + final AtomicInteger count = new AtomicInteger(0); + final Map map = mapOf("1", 1, "2", 2, "3", 3); + final Tuple2, ? extends Map> results = map.partition(entry -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(mapOf("1", 1, "2", 2, "3", 3)); + assertThat(results._2).isEmpty(); + assertThat(count.get()).isEqualTo(3); + } + // -- spliterator @Test diff --git a/src/test/java/io/vavr/collection/AbstractSetTest.java b/src/test/java/io/vavr/collection/AbstractSetTest.java index 687f11b200..bd4c6a435b 100644 --- a/src/test/java/io/vavr/collection/AbstractSetTest.java +++ b/src/test/java/io/vavr/collection/AbstractSetTest.java @@ -18,10 +18,12 @@ */ package io.vavr.collection; +import io.vavr.Tuple2; import org.junit.Test; import java.math.BigDecimal; import java.util.Spliterator; +import java.util.concurrent.atomic.AtomicInteger; public abstract class AbstractSetTest extends AbstractTraversableRangeTest { @@ -189,6 +191,20 @@ public void shouldRemoveElement() { assertThat(empty().remove(5)).isEqualTo(empty()); } + // -- partition + + @Test + public void shouldPartitionInOneIteration() { + final AtomicInteger count = new AtomicInteger(0); + final Tuple2, ? extends Set> results = of(1, 2, 3).partition(i -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(of(1, 2, 3)); + assertThat(results._2).isEqualTo(of()); + assertThat(count.get()).isEqualTo(3); + } + // -- removeAll @Test @@ -227,7 +243,7 @@ public void shouldReturnSameSetWhenEmptyUnionNonEmpty() { assertThat(empty().union(set)).isSameAs(set); } } - + @Test public void shouldReturnSameSetWhenNonEmptyUnionEmpty() { final Set set = of(1, 2); diff --git a/src/test/java/io/vavr/collection/ArrayTest.java b/src/test/java/io/vavr/collection/ArrayTest.java index 27aac0918a..37b6b5fb26 100644 --- a/src/test/java/io/vavr/collection/ArrayTest.java +++ b/src/test/java/io/vavr/collection/ArrayTest.java @@ -27,6 +27,7 @@ import java.math.BigDecimal; import java.util.ArrayList; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collector; @@ -243,6 +244,20 @@ public void shouldThrowExceptionWhenGetIndexEqualToLength() { .isInstanceOf(IndexOutOfBoundsException.class).hasMessage("get(1)"); } + // -- partition + + @Test + public void shouldPartitionInOneIteration() { + final AtomicInteger count = new AtomicInteger(0); + final Tuple2, Array> results = of(1, 2, 3).partition(i -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(of(1, 2, 3)); + assertThat(results._2).isEqualTo(of()); + assertThat(count.get()).isEqualTo(3); + } + // -- transform() @Test diff --git a/src/test/java/io/vavr/collection/CharSeqTest.java b/src/test/java/io/vavr/collection/CharSeqTest.java index 33dff62e9e..343b9e90b6 100644 --- a/src/test/java/io/vavr/collection/CharSeqTest.java +++ b/src/test/java/io/vavr/collection/CharSeqTest.java @@ -273,6 +273,20 @@ public void shouldCaclNonemptyOrElseSupplier() { assertThat(src.orElse(() -> CharSeq.of("12"))).isSameAs(src); } + // -- partition + + @Test + public void shouldPartitionInOneIteration() { + final AtomicInteger count = new AtomicInteger(0); + final Tuple2 results = CharSeq.of('1', '2', '3').partition(c -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(CharSeq.of('1', '2', '3')); + assertThat(results._2).isEqualTo(CharSeq.of()); + assertThat(count.get()).isEqualTo(3); + } + // -- patch @Test diff --git a/src/test/java/io/vavr/collection/ListTest.java b/src/test/java/io/vavr/collection/ListTest.java index 0f3284efa9..0eb2486284 100644 --- a/src/test/java/io/vavr/collection/ListTest.java +++ b/src/test/java/io/vavr/collection/ListTest.java @@ -29,6 +29,7 @@ import java.util.ArrayList; import java.util.NoSuchElementException; import java.util.Spliterator; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collector; @@ -240,6 +241,20 @@ public void shouldReturnSelfWhenIterableIsInstanceOfListView() { assertThat(target).isSameAs(source.getDelegate()); } + // -- partition + + @Test + public void shouldPartitionInOneIteration() { + final AtomicInteger count = new AtomicInteger(0); + final Tuple2, List> results = of(1, 2, 3).partition(i -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(of(1, 2, 3)); + assertThat(results._2).isEqualTo(of()); + assertThat(count.get()).isEqualTo(3); + } + // -- peek @Test(expected = NoSuchElementException.class) diff --git a/src/test/java/io/vavr/collection/QueueTest.java b/src/test/java/io/vavr/collection/QueueTest.java index 4f17505f26..168ddf0ef0 100644 --- a/src/test/java/io/vavr/collection/QueueTest.java +++ b/src/test/java/io/vavr/collection/QueueTest.java @@ -27,6 +27,7 @@ import java.util.ArrayList; import java.util.NoSuchElementException; import java.util.Spliterator; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collector; @@ -236,6 +237,20 @@ public void shouldReturnSelfWhenIterableIsInstanceOfListView() { assertThat(target).isSameAs(source.getDelegate()); } + // -- partition + + @Test + public void shouldPartitionInOneIteration() { + final AtomicInteger count = new AtomicInteger(0); + final Tuple2, Queue> results = of(1, 2, 3).partition(i -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(of(1, 2, 3)); + assertThat(results._2).isEqualTo(of()); + assertThat(count.get()).isEqualTo(3); + } + // -- peek @Test(expected = NoSuchElementException.class) diff --git a/src/test/java/io/vavr/collection/StreamTest.java b/src/test/java/io/vavr/collection/StreamTest.java index 4e7c318e3f..a6faabbedb 100644 --- a/src/test/java/io/vavr/collection/StreamTest.java +++ b/src/test/java/io/vavr/collection/StreamTest.java @@ -28,6 +28,7 @@ import java.math.BigDecimal; import java.util.ArrayList; import java.util.Spliterator; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -469,6 +470,20 @@ public void shouldFlatMapInfiniteTraversable() { .isEqualTo(Stream.of(1, 2, 2, 4, 3, 6, 4)); } + // -- partition + + @Test + public void shouldPartitionInTwoIterations() { + final AtomicInteger count = new AtomicInteger(0); + final Tuple2, Stream> results = Stream.of(1, 2, 3).partition(i -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(of(1, 2, 3)); + assertThat(results._2).isEqualTo(of()); + assertThat(count.get()).isEqualTo(6); + } + // -- peek @Override diff --git a/src/test/java/io/vavr/collection/VectorTest.java b/src/test/java/io/vavr/collection/VectorTest.java index 14e2e6709c..2bc684a2c1 100644 --- a/src/test/java/io/vavr/collection/VectorTest.java +++ b/src/test/java/io/vavr/collection/VectorTest.java @@ -28,6 +28,7 @@ import java.io.InvalidObjectException; import java.math.BigDecimal; import java.util.ArrayList; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collector; @@ -235,6 +236,21 @@ public void shouldReturnSelfWhenIterableIsInstanceOfListView() { assertThat(target).isSameAs(source.getDelegate()); } + // -- partition + + @Test + public void shouldPartitionInOneIteration() { + final AtomicInteger count = new AtomicInteger(0); + final Vector values = ofAll(1, 2, 3); + final Tuple2, Vector> results = values.partition(v -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(ofAll(1, 2, 3)); + assertThat(results._2).isEmpty(); + assertThat(count.get()).isEqualTo(3); + } + // -- primitives @Test From 5829a64f565f4bb2f29d0cb6a7ff0493e09a2567 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Sun, 17 May 2020 14:25:15 +0200 Subject: [PATCH 05/15] Test that Stream.partition() is lazy --- .../java/io/vavr/collection/StreamTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/test/java/io/vavr/collection/StreamTest.java b/src/test/java/io/vavr/collection/StreamTest.java index a6faabbedb..f50f7a0875 100644 --- a/src/test/java/io/vavr/collection/StreamTest.java +++ b/src/test/java/io/vavr/collection/StreamTest.java @@ -484,6 +484,25 @@ public void shouldPartitionInTwoIterations() { assertThat(count.get()).isEqualTo(6); } + @Test + public void shouldPartitionLazily() { + final java.util.Set itemsCalled = new java.util.HashSet<>(); + + final Stream infiniteStream = Stream.iterate(0, i -> i + 1); + assertThat(itemsCalled).isEmpty(); + + final Tuple2, Stream> results = infiniteStream.partition(i -> { + itemsCalled.add(i); + return i % 2 == 0; + }); + assertThat(itemsCalled).containsExactly(0, 1); + assertThat(results._1.head()).isEqualTo(0); + assertThat(results._2.head()).isEqualTo(1); + assertThat(results._1.take(3)).isEqualTo(of(0, 2, 4)); + assertThat(results._2.take(3)).isEqualTo(of(1, 3, 5)); + assertThat(itemsCalled).containsExactly(0, 1, 2, 3, 4, 5); + } + // -- peek @Override From 10ff3d443f02d713c948848ab17652fa11c6f46c Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Sun, 17 May 2020 22:44:00 +0200 Subject: [PATCH 06/15] Create Iterator.duplicate() and add tests --- .../java/io/vavr/collection/Iterator.java | 69 ++++++++++++ .../java/io/vavr/collection/IteratorTest.java | 101 ++++++++++++++++++ 2 files changed, 170 insertions(+) diff --git a/src/main/java/io/vavr/collection/Iterator.java b/src/main/java/io/vavr/collection/Iterator.java index d8a7649435..99a41a7c13 100644 --- a/src/main/java/io/vavr/collection/Iterator.java +++ b/src/main/java/io/vavr/collection/Iterator.java @@ -23,6 +23,7 @@ import java.math.BigDecimal; import java.util.*; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.*; import static io.vavr.collection.BigDecimalHelper.areEqual; @@ -1958,6 +1959,74 @@ default Tuple2, Iterator> span(Predicate predicate) { } } + /** + * Creates two new iterators that both iterates over the same elements as + * this iterator and in the same order. The duplicate iterators are + * considered equal if they are positioned at the same element. + *

+ * Given that most methods on iterators will make the original iterator + * unfit for further use, this methods provides a reliable way of calling + * multiple such methods on an iterator. + * + * @return a pair of iterators + */ + default Tuple2, Iterator> duplicate() { + final java.util.Queue gap = new java.util.LinkedList<>(); + final AtomicReference> ahead = new AtomicReference<>(); + class Partner implements Iterator { + + @Override + public synchronized boolean hasNext() { + return (this != ahead.get() && !gap.isEmpty()) || Iterator.this.hasNext(); + } + + @Override + public synchronized T next() { + if (gap.isEmpty()) { + ahead.set(this); + } + if (this == ahead.get()) { + final T element = Iterator.this.next(); + gap.add(element); + return element; + } else { + return gap.poll(); + } + } + + private boolean hasSame(java.util.Queue queue) { + return gap == queue; + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (obj == null) { + return false; + } + if (obj.getClass().isAssignableFrom(Partner.class)) { + @SuppressWarnings("unchecked") + final Partner that = (Partner) obj; + /* + * Two iterators are equal if and only if they are partners + * (use the same queue for iteration) and they iterated + * over the same number of elements, i.e. no gaps. + */ + return that.hasSame(gap) && gap.isEmpty(); + } + return super.equals(obj); + } + + @Override + public int hashCode() { + return gap.hashCode(); + } + } + return Tuple.of(new Partner(), new Partner()); + } + @Override default String stringPrefix() { return "Iterator"; diff --git a/src/test/java/io/vavr/collection/IteratorTest.java b/src/test/java/io/vavr/collection/IteratorTest.java index 8416fa3e33..0b3b9b56db 100644 --- a/src/test/java/io/vavr/collection/IteratorTest.java +++ b/src/test/java/io/vavr/collection/IteratorTest.java @@ -521,6 +521,107 @@ public void shouldRemoveDuplicatesOnDistinct() { assertThat(of(1, 2, 1, 3, 3).distinct().toList()).isEqualTo(List.of(1, 2, 3)); } + // -- duplicate + + @Test + public void shouldDuplicateIteratorsWithCorrectValues() { + final Tuple2, Iterator> partners = of(1, 2, 3).duplicate(); + + assertThat(partners._1).isEqualTo(of(1, 2, 3)); + assertThat(partners._2).isEqualTo(of(1, 2, 3)); + } + + @Test + public void shouldDuplicateIteratorsWithCorrectHashCodeAndEquals() { + final Tuple2, Iterator> partners = of(1, 2, 3).duplicate(); + + assertThat(partners._1.equals(partners._1)).isTrue(); + assertThat(partners._1.equals(partners._2)).isTrue(); + assertThat(partners._2.equals(partners._1)).isTrue(); + + assertThat(partners._1.equals(null)).isFalse(); + assertThat(partners._1.equals(of(1, 2, 3))).isFalse(); + assertThat(partners._2.equals(of(1, 2, 3))).isFalse(); + assertThat(partners._2.equals(of("1", "2", "3"))).isFalse(); + + assertThat(partners._1.hashCode()).isEqualTo(partners._2.hashCode()); + assertThat(partners._1.hashCode()).isNotEqualTo(of(1, 2, 3).hashCode()); + } + + @Test + public void shouldCalculateEqualsForDuplicateIteratorsBasedOnGap() { + final Tuple2, Iterator> partners = of(1, 2, 3).duplicate(); + final Iterator ahead = partners._1; + final Iterator behind = partners._2; + assertThat(ahead.equals(behind)).isTrue(); + + for (final int i : of(1, 2, 3)) { + assertThat(ahead.hasNext()).isTrue(); + assertThat(ahead.next()).isEqualTo(i); + assertThat(ahead.equals(behind)).as("Two iterators are different when gap exists").isFalse(); + + assertThat(behind.hasNext()).isTrue(); + assertThat(behind.next()).isEqualTo(i); + assertThat(ahead.equals(behind)).as("Two iterators are equal when no gap").isTrue(); + } + } + + @Test(timeout = 5_000L) // avoid endless test caused by infinite iterator + public void shouldDuplicateIteratorsLazily() { + final java.util.List itemsCalled = new java.util.ArrayList<>(); + + // Given an infinite iterator + final Iterator iterator = Iterator.iterate(1, i -> { + itemsCalled.add(i); + return i + 1; + }); + + // When duplicating it + final Tuple2, Iterator> partners = iterator.duplicate(); + + // Then the duplication is done lazily + assertThat(iterator.hasNext()).isTrue(); + assertThat(itemsCalled).as("Duplicating iterators should be done lazily.").isEmpty(); + + // When retrieving more elements + for (final int i : of(1, 2, 3)) { + assertThat(partners._1.hasNext()).isTrue(); + assertThat(partners._2.hasNext()).isTrue(); + assertThat(partners._1.next()).isEqualTo(i); + assertThat(partners._2.next()).isEqualTo(i); + } + + // Then the original iterator is called, once per next() + assertThat(itemsCalled).containsExactly(1, 2); + } + + @Test(timeout = 5_000L) // avoid endless test caused by infinite iterator + public void shouldLetBehindIteratorBecomeAheadIteratorWhenItMovesForward() { + final java.util.List itemsCalled = new java.util.ArrayList<>(); + + // Given an infinite iterator + final Iterator iterator = Iterator.iterate(1, i -> { + itemsCalled.add(i); + return i + 1; + }); + + // When duplicating it + final Tuple2, Iterator> partners = iterator.duplicate(); + // And move forward one iterator + assertThat(partners._1.hasNext()).isTrue(); + assertThat(partners._1.next()).isEqualTo(1); + // Then we have one iterator "ahead" and the other iterator "behind" + + // When moving forward the "behind" iterator + // Then it becomes the new iterator ahead because `duplicate()` should + // be able to change the ahead iterator + for (final int i : of(1, 2, 3, 4)) { + assertThat(partners._2.hasNext()).isTrue(); + assertThat(partners._2.next()).isEqualTo(i); + } + assertThat(itemsCalled).containsExactly(1, 2, 3); + } + // -- groupBy @Override From 7316baa75b1d1f91762873860811940c0bef9860 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Tue, 19 May 2020 22:26:16 +0200 Subject: [PATCH 07/15] Change the implementation of Iterator.partition() --- .../java/io/vavr/collection/Iterator.java | 12 ++----- .../java/io/vavr/collection/IteratorTest.java | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/main/java/io/vavr/collection/Iterator.java b/src/main/java/io/vavr/collection/Iterator.java index 99a41a7c13..cc35a6cdf9 100644 --- a/src/main/java/io/vavr/collection/Iterator.java +++ b/src/main/java/io/vavr/collection/Iterator.java @@ -1723,16 +1723,8 @@ default Tuple2, Iterator> partition(Predicate predicat if (!hasNext()) { return Tuple.of(empty(), empty()); } else { - final java.util.List first = new ArrayList<>(); - final java.util.List second = new ArrayList<>(); - for (T t : this) { - if (predicate.test(t)) { - first.add(t); - } else { - second.add(t); - } - } - return Tuple.of(ofAll(first), ofAll(second)); + final Tuple2, Iterator> dup = duplicate(); + return Tuple.of(dup._1.filter(predicate), dup._2.filterNot(predicate)); } } diff --git a/src/test/java/io/vavr/collection/IteratorTest.java b/src/test/java/io/vavr/collection/IteratorTest.java index 0b3b9b56db..2e2e952d73 100644 --- a/src/test/java/io/vavr/collection/IteratorTest.java +++ b/src/test/java/io/vavr/collection/IteratorTest.java @@ -834,6 +834,41 @@ public void shouldReturnSomeOnNextOptionWhenIteratorOfOneElement() { assertThat(Iterator.of(1).nextOption()).isEqualTo(Option.some(1)); } + // -- partition() + + @Test + public void shouldPartition() { + final Tuple2, Iterator> partitions = of(1, 2, 3).partition(i -> i % 2 == 0); + assertThat(partitions._1).isEqualTo(of(2)); + assertThat(partitions._2).isEqualTo(of(1, 3)); + } + + @Test(timeout = 5_000L) // avoid endless test caused by infinite iterator + public void shouldPartitionLazily() { + final java.util.List itemsCalled = new java.util.ArrayList<>(); + + // Given an infinite iterator + final Iterator iterator = Iterator.iterate(1, i -> { + itemsCalled.add(i); + return i + 1; + }); + + // When partitioning it + // Then the partitioning is done lazily (otherwise the test will timeout) + final Tuple2, Iterator> partitions = iterator.partition(i -> i % 2 == 0); + assertThat(itemsCalled).isEmpty(); + + // When moving forwards iterators + // Then the moves are done as expected + assertThat(partitions._1.hasNext()).isTrue(); + assertThat(partitions._1.next()).isEqualTo(2); + for (int i : of(1, 3, 5)) { + assertThat(partitions._2.hasNext()).isTrue(); + assertThat(partitions._2.next()).isEqualTo(i); + } + assertThat(itemsCalled).containsExactly(1, 2, 3, 4); + } + // -- .toString() @Test From 0dce6b92641b6657f257ae95d46940ec26c5f9d6 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Wed, 20 May 2020 20:52:02 +0200 Subject: [PATCH 08/15] Fix Set --- src/main/java/io/vavr/collection/BitSet.java | 3 +-- .../java/io/vavr/collection/Collections.java | 15 ++++++++++++++- src/main/java/io/vavr/collection/HashSet.java | 6 ++---- .../java/io/vavr/collection/LinkedHashSet.java | 4 +--- .../java/io/vavr/collection/Traversable.java | 18 +++++++++--------- src/main/java/io/vavr/collection/TreeSet.java | 4 +--- 6 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/main/java/io/vavr/collection/BitSet.java b/src/main/java/io/vavr/collection/BitSet.java index acb716fbd7..4aa3c5fc6d 100644 --- a/src/main/java/io/vavr/collection/BitSet.java +++ b/src/main/java/io/vavr/collection/BitSet.java @@ -831,8 +831,7 @@ public BitSet scan(T zero, BiFunction oper @Override public Tuple2, BitSet> partition(Predicate predicate) { - Objects.requireNonNull(predicate, "predicate is null"); - return iterator().partition(predicate).map(this::createFromAll, this::createFromAll); + return Collections.partition(this, this::createFromAll, predicate); } @Override diff --git a/src/main/java/io/vavr/collection/Collections.java b/src/main/java/io/vavr/collection/Collections.java index 991dfbf876..9ddd1776e2 100644 --- a/src/main/java/io/vavr/collection/Collections.java +++ b/src/main/java/io/vavr/collection/Collections.java @@ -18,6 +18,8 @@ */ package io.vavr.collection; +import io.vavr.Tuple; +import io.vavr.Tuple2; import io.vavr.collection.JavaConverters.ChangePolicy; import io.vavr.collection.JavaConverters.ListView; import io.vavr.control.Option; @@ -294,6 +296,17 @@ static > U mapKeys(Map source, U zero, Func }); } + static , T> Tuple2 partition(C collection, Function, C> creator, + Predicate predicate) { + Objects.requireNonNull(predicate, "predicate is null"); + final java.util.List left = new java.util.ArrayList<>(); + final java.util.List right = new java.util.ArrayList<>(); + for (T element : collection) { + (predicate.test(element) ? left : right).add(element); + } + return Tuple.of(creator.apply(left), creator.apply(right)); + } + @SuppressWarnings("unchecked") static , T> C removeAll(C source, Iterable elements) { Objects.requireNonNull(elements, "elements is null"); @@ -556,7 +569,7 @@ private static IterableWithSize withSizeTraversable(Iterable return new IterableWithSize<>(iterable, ((Traversable) iterable).size()); } } - + static class IterableWithSize { private final Iterable iterable; private final int size; diff --git a/src/main/java/io/vavr/collection/HashSet.java b/src/main/java/io/vavr/collection/HashSet.java index 04e68b4508..73b37874f9 100644 --- a/src/main/java/io/vavr/collection/HashSet.java +++ b/src/main/java/io/vavr/collection/HashSet.java @@ -668,7 +668,7 @@ public boolean isAsync() { public boolean isEmpty() { return tree.isEmpty(); } - + /** * A {@code HashSet} is computed eagerly. * @@ -730,9 +730,7 @@ public HashSet orElse(Supplier> supplier) { @Override public Tuple2, HashSet> partition(Predicate predicate) { - Objects.requireNonNull(predicate, "predicate is null"); - final Tuple2, Iterator> p = iterator().partition(predicate); - return Tuple.of(HashSet.ofAll(p._1), HashSet.ofAll(p._2)); + return Collections.partition(this, HashSet::ofAll, predicate); } @Override diff --git a/src/main/java/io/vavr/collection/LinkedHashSet.java b/src/main/java/io/vavr/collection/LinkedHashSet.java index 9e5a7911e5..c28e7d8d6e 100644 --- a/src/main/java/io/vavr/collection/LinkedHashSet.java +++ b/src/main/java/io/vavr/collection/LinkedHashSet.java @@ -751,9 +751,7 @@ public LinkedHashSet orElse(Supplier> supplie @Override public Tuple2, LinkedHashSet> partition(Predicate predicate) { - Objects.requireNonNull(predicate, "predicate is null"); - final Tuple2, Iterator> p = iterator().partition(predicate); - return Tuple.of(LinkedHashSet.ofAll(p._1), LinkedHashSet.ofAll(p._2)); + return Collections.partition(this, LinkedHashSet::ofAll, predicate); } @Override diff --git a/src/main/java/io/vavr/collection/Traversable.java b/src/main/java/io/vavr/collection/Traversable.java index ed8b994cdf..0d88402997 100644 --- a/src/main/java/io/vavr/collection/Traversable.java +++ b/src/main/java/io/vavr/collection/Traversable.java @@ -164,7 +164,7 @@ public interface Traversable extends Iterable, Foldable, io.vavr.Value< static Traversable narrow(Traversable traversable) { return (Traversable) traversable; } - + /** * Matches each element with a unique key that you extract from it. * If the same key is present twice, the function will return {@code None}. @@ -223,7 +223,7 @@ default Option average() { throw new UnsupportedOperationException("not numeric", x); } } - + /** * Collects all elements that are in the domain of the given {@code partialFunction} by mapping the elements to type {@code R}. *

@@ -341,7 +341,7 @@ default int count(Predicate predicate) { Traversable dropUntil(Predicate predicate); /** - * Drops elements while the predicate holds for the current element. + * Drops elements while the predicate holds for the current element. *

* Note: This is essentially the same as {@code dropUntil(predicate.negate())}. * It is intended to be used with method references, which cannot be negated directly. @@ -370,7 +370,7 @@ default int count(Predicate predicate) { *

  • contain the same elements
    • *
  • have the same element order, if the collections are of type Seq
    • * - * + * * Two Map/Multimap elements, resp. entries, (key1, value1) and (key2, value2) are equal, * if the keys are equal and the values are equal. *

    @@ -685,7 +685,7 @@ default T get() { default Option headOption() { return isEmpty() ? Option.none() : Option.some(head()); } - + /** * Returns the hash code of this collection. *
    @@ -1055,7 +1055,7 @@ default > Option minBy(Function @@ -1372,7 +1372,7 @@ default Option reduceRightOption(BiFunction Traversable scanRight(U zero, BiFunction operation); - + /** * Returns the single element of this Traversable or throws, if this is empty or contains more than one element. * @@ -1536,7 +1536,7 @@ default Number sum() { } } } - + /** * Drops the first element of a non-empty Traversable. * @@ -1666,7 +1666,7 @@ default Number sum() { * @throws NullPointerException if {@code that} is null */ Traversable> zipAll(Iterable that, T thisElem, U thatElem); - + /** * Returns a traversable formed from this traversable and another Iterable collection by mapping elements. * If one of the two iterables is longer than the other, its remaining elements are ignored. diff --git a/src/main/java/io/vavr/collection/TreeSet.java b/src/main/java/io/vavr/collection/TreeSet.java index 50e4f04f02..688c8a3005 100644 --- a/src/main/java/io/vavr/collection/TreeSet.java +++ b/src/main/java/io/vavr/collection/TreeSet.java @@ -800,9 +800,7 @@ public TreeSet orElse(Supplier> supplier) { @Override public Tuple2, TreeSet> partition(Predicate predicate) { - Objects.requireNonNull(predicate, "predicate is null"); - return iterator().partition(predicate).map(i1 -> TreeSet.ofAll(tree.comparator(), i1), - i2 -> TreeSet.ofAll(tree.comparator(), i2)); + return Collections.partition(this, values -> TreeSet.ofAll(tree.comparator(), values), predicate); } @Override From d48644664a213bc52b486016aa3910cb3324fc08 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Wed, 20 May 2020 21:30:05 +0200 Subject: [PATCH 09/15] Fix Map --- src/main/java/io/vavr/collection/Maps.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/vavr/collection/Maps.java b/src/main/java/io/vavr/collection/Maps.java index ae2af545bc..179e687d26 100644 --- a/src/main/java/io/vavr/collection/Maps.java +++ b/src/main/java/io/vavr/collection/Maps.java @@ -201,8 +201,12 @@ static > M ofStream(M map, java.util.stream.Stream< static > Tuple2 partition(M map, OfEntries ofEntries, Predicate> predicate) { Objects.requireNonNull(predicate, "predicate is null"); - final Tuple2>, Iterator>> p = map.iterator().partition(predicate); - return Tuple.of(ofEntries.apply(p._1), ofEntries.apply(p._2)); + final java.util.List> left = new java.util.ArrayList<>(); + final java.util.List> right = new java.util.ArrayList<>(); + for (Tuple2 entry : map) { + (predicate.test(entry) ? left : right).add(entry); + } + return Tuple.of(ofEntries.apply(left), ofEntries.apply(right)); } static > M peek(M map, Consumer> action) { From b75a1b62f9c252ca016d4af1ff79994cd39f8cfa Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Wed, 20 May 2020 22:36:34 +0200 Subject: [PATCH 10/15] Fix Multimap --- .../java/io/vavr/collection/AbstractMultimap.java | 8 ++++++-- .../io/vavr/collection/AbstractMultimapTest.java | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/vavr/collection/AbstractMultimap.java b/src/main/java/io/vavr/collection/AbstractMultimap.java index f3dd468130..ba933d9199 100644 --- a/src/main/java/io/vavr/collection/AbstractMultimap.java +++ b/src/main/java/io/vavr/collection/AbstractMultimap.java @@ -472,8 +472,12 @@ public M orElse(Supplier>> supplier) { @Override public Tuple2 partition(Predicate> predicate) { Objects.requireNonNull(predicate, "predicate is null"); - final Tuple2>, Iterator>> p = iterator().partition(predicate); - return Tuple.of((M) createFromEntries(p._1), (M) createFromEntries(p._2)); + final java.util.List> left = new java.util.ArrayList<>(); + final java.util.List> right = new java.util.ArrayList<>(); + for (Tuple2 entry : this) { + (predicate.test(entry) ? left : right).add(entry); + } + return Tuple.of((M) createFromEntries(left), (M) createFromEntries(right)); } @SuppressWarnings("unchecked") diff --git a/src/test/java/io/vavr/collection/AbstractMultimapTest.java b/src/test/java/io/vavr/collection/AbstractMultimapTest.java index d5a32f9aa1..20e3e5b86b 100644 --- a/src/test/java/io/vavr/collection/AbstractMultimapTest.java +++ b/src/test/java/io/vavr/collection/AbstractMultimapTest.java @@ -809,6 +809,20 @@ public void shouldPartitionIntsInOddAndEvenHavingOddAndEvenNumbers() { mapOfTuples(Tuple.of(1, 2), Tuple.of(3, 4)))); } + @Test + @SuppressWarnings("unchecked") + public void shouldPartitionInOneIteration() { + final AtomicInteger count = new AtomicInteger(0); + final Multimap map = mapOfTuples(Tuple.of("1", 1), Tuple.of("2", 2), Tuple.of("3", 3)); + final Tuple2, ? extends Multimap> results = map.partition(entry -> { + count.incrementAndGet(); + return true; + }); + assertThat(results._1).isEqualTo(mapOfTuples(Tuple.of("1", 1), Tuple.of("2", 2), Tuple.of("3", 3))); + assertThat(results._2).isEmpty(); + assertThat(count.get()).isEqualTo(3); + } + // -- put @Test From 64be4e4c33604e929d45ea116ab8b2d4cb090c9c Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Fri, 22 May 2020 20:50:38 +0200 Subject: [PATCH 11/15] Move duplicate to IteratorModule --- .../java/io/vavr/collection/Iterator.java | 139 +++++++++--------- .../java/io/vavr/collection/IteratorTest.java | 10 +- 2 files changed, 76 insertions(+), 73 deletions(-) diff --git a/src/main/java/io/vavr/collection/Iterator.java b/src/main/java/io/vavr/collection/Iterator.java index cc35a6cdf9..70a68a892d 100644 --- a/src/main/java/io/vavr/collection/Iterator.java +++ b/src/main/java/io/vavr/collection/Iterator.java @@ -1723,7 +1723,7 @@ default Tuple2, Iterator> partition(Predicate predicat if (!hasNext()) { return Tuple.of(empty(), empty()); } else { - final Tuple2, Iterator> dup = duplicate(); + final Tuple2, Iterator> dup = IteratorModule.duplicate(this); return Tuple.of(dup._1.filter(predicate), dup._2.filterNot(predicate)); } } @@ -1951,73 +1951,6 @@ default Tuple2, Iterator> span(Predicate predicate) { } } - /** - * Creates two new iterators that both iterates over the same elements as - * this iterator and in the same order. The duplicate iterators are - * considered equal if they are positioned at the same element. - *

    - * Given that most methods on iterators will make the original iterator - * unfit for further use, this methods provides a reliable way of calling - * multiple such methods on an iterator. - * - * @return a pair of iterators - */ - default Tuple2, Iterator> duplicate() { - final java.util.Queue gap = new java.util.LinkedList<>(); - final AtomicReference> ahead = new AtomicReference<>(); - class Partner implements Iterator { - - @Override - public synchronized boolean hasNext() { - return (this != ahead.get() && !gap.isEmpty()) || Iterator.this.hasNext(); - } - - @Override - public synchronized T next() { - if (gap.isEmpty()) { - ahead.set(this); - } - if (this == ahead.get()) { - final T element = Iterator.this.next(); - gap.add(element); - return element; - } else { - return gap.poll(); - } - } - - private boolean hasSame(java.util.Queue queue) { - return gap == queue; - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (obj == null) { - return false; - } - if (obj.getClass().isAssignableFrom(Partner.class)) { - @SuppressWarnings("unchecked") - final Partner that = (Partner) obj; - /* - * Two iterators are equal if and only if they are partners - * (use the same queue for iteration) and they iterated - * over the same number of elements, i.e. no gaps. - */ - return that.hasSame(gap) && gap.isEmpty(); - } - return super.equals(obj); - } - - @Override - public int hashCode() { - return gap.hashCode(); - } - } - return Tuple.of(new Partner(), new Partner()); - } @Override default String stringPrefix() { @@ -2249,6 +2182,76 @@ public String toString() { } } +interface IteratorModule { + /** + * Creates two new iterators that both iterates over the same elements as + * this iterator and in the same order. The duplicate iterators are + * considered equal if they are positioned at the same element. + *

    + * Given that most methods on iterators will make the original iterator + * unfit for further use, this methods provides a reliable way of calling + * multiple such methods on an iterator. + * + * @return a pair of iterators + */ + static Tuple2, Iterator> duplicate(Iterator iterator) { + final java.util.Queue gap = new java.util.LinkedList<>(); + final AtomicReference> ahead = new AtomicReference<>(); + class Partner implements Iterator { + + @Override + public synchronized boolean hasNext() { + return (this != ahead.get() && !gap.isEmpty()) || iterator.hasNext(); + } + + @Override + public synchronized T next() { + if (gap.isEmpty()) { + ahead.set(this); + } + if (this == ahead.get()) { + final T element = iterator.next(); + gap.add(element); + return element; + } else { + return gap.poll(); + } + } + + private boolean hasSame(java.util.Queue queue) { + return gap == queue; + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (obj == null) { + return false; + } + if (obj.getClass().isAssignableFrom(Partner.class)) { + @SuppressWarnings("unchecked") + final Partner that = (Partner) obj; + /* + * Two iterators are equal if and only if they are partners + * (use the same queue for iteration) and they iterated + * over the same number of elements, i.e. no gaps. + */ + return that.hasSame(gap) && gap.isEmpty(); + } + return super.equals(obj); + } + + @Override + public int hashCode() { + return gap.hashCode(); + } + } + return Tuple.of(new Partner(), new Partner()); + } +} + final class ConcatIterator implements Iterator { private static final class Iterators { diff --git a/src/test/java/io/vavr/collection/IteratorTest.java b/src/test/java/io/vavr/collection/IteratorTest.java index 2e2e952d73..1d0f9b156a 100644 --- a/src/test/java/io/vavr/collection/IteratorTest.java +++ b/src/test/java/io/vavr/collection/IteratorTest.java @@ -525,7 +525,7 @@ public void shouldRemoveDuplicatesOnDistinct() { @Test public void shouldDuplicateIteratorsWithCorrectValues() { - final Tuple2, Iterator> partners = of(1, 2, 3).duplicate(); + final Tuple2, Iterator> partners = IteratorModule.duplicate(of(1, 2, 3)); assertThat(partners._1).isEqualTo(of(1, 2, 3)); assertThat(partners._2).isEqualTo(of(1, 2, 3)); @@ -533,7 +533,7 @@ public void shouldDuplicateIteratorsWithCorrectValues() { @Test public void shouldDuplicateIteratorsWithCorrectHashCodeAndEquals() { - final Tuple2, Iterator> partners = of(1, 2, 3).duplicate(); + final Tuple2, Iterator> partners = IteratorModule.duplicate(of(1, 2, 3)); assertThat(partners._1.equals(partners._1)).isTrue(); assertThat(partners._1.equals(partners._2)).isTrue(); @@ -550,7 +550,7 @@ public void shouldDuplicateIteratorsWithCorrectHashCodeAndEquals() { @Test public void shouldCalculateEqualsForDuplicateIteratorsBasedOnGap() { - final Tuple2, Iterator> partners = of(1, 2, 3).duplicate(); + final Tuple2, Iterator> partners = IteratorModule.duplicate(of(1, 2, 3)); final Iterator ahead = partners._1; final Iterator behind = partners._2; assertThat(ahead.equals(behind)).isTrue(); @@ -577,7 +577,7 @@ public void shouldDuplicateIteratorsLazily() { }); // When duplicating it - final Tuple2, Iterator> partners = iterator.duplicate(); + final Tuple2, Iterator> partners = IteratorModule.duplicate(iterator); // Then the duplication is done lazily assertThat(iterator.hasNext()).isTrue(); @@ -606,7 +606,7 @@ public void shouldLetBehindIteratorBecomeAheadIteratorWhenItMovesForward() { }); // When duplicating it - final Tuple2, Iterator> partners = iterator.duplicate(); + final Tuple2, Iterator> partners = IteratorModule.duplicate(iterator); // And move forward one iterator assertThat(partners._1.hasNext()).isTrue(); assertThat(partners._1.next()).isEqualTo(1); From 87e5bab548e5080ed3dbeaadb9f5fd96f85142f8 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Fri, 22 May 2020 21:01:13 +0200 Subject: [PATCH 12/15] Remove synchronized keyword --- src/main/java/io/vavr/collection/Iterator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/vavr/collection/Iterator.java b/src/main/java/io/vavr/collection/Iterator.java index 70a68a892d..af7c8172bf 100644 --- a/src/main/java/io/vavr/collection/Iterator.java +++ b/src/main/java/io/vavr/collection/Iterator.java @@ -2200,12 +2200,12 @@ static Tuple2, Iterator> duplicate(Iterator iterator) { class Partner implements Iterator { @Override - public synchronized boolean hasNext() { + public boolean hasNext() { return (this != ahead.get() && !gap.isEmpty()) || iterator.hasNext(); } @Override - public synchronized T next() { + public T next() { if (gap.isEmpty()) { ahead.set(this); } From 17c6f4c0355cf19bb4d974804d6c5aaf45e3f0b3 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Fri, 22 May 2020 21:05:45 +0200 Subject: [PATCH 13/15] Remove hashCode and equals --- .../java/io/vavr/collection/Iterator.java | 30 ---------------- .../java/io/vavr/collection/IteratorTest.java | 35 ------------------- 2 files changed, 65 deletions(-) diff --git a/src/main/java/io/vavr/collection/Iterator.java b/src/main/java/io/vavr/collection/Iterator.java index af7c8172bf..289b06fac9 100644 --- a/src/main/java/io/vavr/collection/Iterator.java +++ b/src/main/java/io/vavr/collection/Iterator.java @@ -2217,36 +2217,6 @@ public T next() { return gap.poll(); } } - - private boolean hasSame(java.util.Queue queue) { - return gap == queue; - } - - @Override - public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (obj == null) { - return false; - } - if (obj.getClass().isAssignableFrom(Partner.class)) { - @SuppressWarnings("unchecked") - final Partner that = (Partner) obj; - /* - * Two iterators are equal if and only if they are partners - * (use the same queue for iteration) and they iterated - * over the same number of elements, i.e. no gaps. - */ - return that.hasSame(gap) && gap.isEmpty(); - } - return super.equals(obj); - } - - @Override - public int hashCode() { - return gap.hashCode(); - } } return Tuple.of(new Partner(), new Partner()); } diff --git a/src/test/java/io/vavr/collection/IteratorTest.java b/src/test/java/io/vavr/collection/IteratorTest.java index 1d0f9b156a..6aba1b717c 100644 --- a/src/test/java/io/vavr/collection/IteratorTest.java +++ b/src/test/java/io/vavr/collection/IteratorTest.java @@ -531,41 +531,6 @@ public void shouldDuplicateIteratorsWithCorrectValues() { assertThat(partners._2).isEqualTo(of(1, 2, 3)); } - @Test - public void shouldDuplicateIteratorsWithCorrectHashCodeAndEquals() { - final Tuple2, Iterator> partners = IteratorModule.duplicate(of(1, 2, 3)); - - assertThat(partners._1.equals(partners._1)).isTrue(); - assertThat(partners._1.equals(partners._2)).isTrue(); - assertThat(partners._2.equals(partners._1)).isTrue(); - - assertThat(partners._1.equals(null)).isFalse(); - assertThat(partners._1.equals(of(1, 2, 3))).isFalse(); - assertThat(partners._2.equals(of(1, 2, 3))).isFalse(); - assertThat(partners._2.equals(of("1", "2", "3"))).isFalse(); - - assertThat(partners._1.hashCode()).isEqualTo(partners._2.hashCode()); - assertThat(partners._1.hashCode()).isNotEqualTo(of(1, 2, 3).hashCode()); - } - - @Test - public void shouldCalculateEqualsForDuplicateIteratorsBasedOnGap() { - final Tuple2, Iterator> partners = IteratorModule.duplicate(of(1, 2, 3)); - final Iterator ahead = partners._1; - final Iterator behind = partners._2; - assertThat(ahead.equals(behind)).isTrue(); - - for (final int i : of(1, 2, 3)) { - assertThat(ahead.hasNext()).isTrue(); - assertThat(ahead.next()).isEqualTo(i); - assertThat(ahead.equals(behind)).as("Two iterators are different when gap exists").isFalse(); - - assertThat(behind.hasNext()).isTrue(); - assertThat(behind.next()).isEqualTo(i); - assertThat(ahead.equals(behind)).as("Two iterators are equal when no gap").isTrue(); - } - } - @Test(timeout = 5_000L) // avoid endless test caused by infinite iterator public void shouldDuplicateIteratorsLazily() { final java.util.List itemsCalled = new java.util.ArrayList<>(); From ba54ca591202b65c8aa1bb87d38dfebd4e6e11b3 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Fri, 22 May 2020 21:40:30 +0200 Subject: [PATCH 14/15] Avoid using isEqualTo --- src/test/java/io/vavr/collection/IteratorTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/java/io/vavr/collection/IteratorTest.java b/src/test/java/io/vavr/collection/IteratorTest.java index 6aba1b717c..60cecdd283 100644 --- a/src/test/java/io/vavr/collection/IteratorTest.java +++ b/src/test/java/io/vavr/collection/IteratorTest.java @@ -525,10 +525,10 @@ public void shouldRemoveDuplicatesOnDistinct() { @Test public void shouldDuplicateIteratorsWithCorrectValues() { - final Tuple2, Iterator> partners = IteratorModule.duplicate(of(1, 2, 3)); + final Tuple2, Iterator> partners = IteratorModule.duplicate(of("1", "2", "3")); - assertThat(partners._1).isEqualTo(of(1, 2, 3)); - assertThat(partners._2).isEqualTo(of(1, 2, 3)); + assertThat(String.join(", ", partners._1)).isEqualTo("1, 2, 3"); + assertThat(String.join(", ", partners._2)).isEqualTo("1, 2, 3"); } @Test(timeout = 5_000L) // avoid endless test caused by infinite iterator @@ -803,9 +803,9 @@ public void shouldReturnSomeOnNextOptionWhenIteratorOfOneElement() { @Test public void shouldPartition() { - final Tuple2, Iterator> partitions = of(1, 2, 3).partition(i -> i % 2 == 0); - assertThat(partitions._1).isEqualTo(of(2)); - assertThat(partitions._2).isEqualTo(of(1, 3)); + final Tuple2, Iterator> partitions = of("1", "2", "3").partition(i -> "2".equals(i)); + assertThat(String.join(", ", partitions._1)).isEqualTo("2"); + assertThat(String.join(", ", partitions._2)).isEqualTo("1, 3"); } @Test(timeout = 5_000L) // avoid endless test caused by infinite iterator From 4481b97acb6b284c77a5448090cd1e2415627146 Mon Sep 17 00:00:00 2001 From: Mincong HUANG Date: Fri, 22 May 2020 21:59:43 +0200 Subject: [PATCH 15/15] Remove redundant tests --- .../java/io/vavr/collection/IteratorTest.java | 68 +------------------ 1 file changed, 1 insertion(+), 67 deletions(-) diff --git a/src/test/java/io/vavr/collection/IteratorTest.java b/src/test/java/io/vavr/collection/IteratorTest.java index 60cecdd283..66241cfb68 100644 --- a/src/test/java/io/vavr/collection/IteratorTest.java +++ b/src/test/java/io/vavr/collection/IteratorTest.java @@ -521,72 +521,6 @@ public void shouldRemoveDuplicatesOnDistinct() { assertThat(of(1, 2, 1, 3, 3).distinct().toList()).isEqualTo(List.of(1, 2, 3)); } - // -- duplicate - - @Test - public void shouldDuplicateIteratorsWithCorrectValues() { - final Tuple2, Iterator> partners = IteratorModule.duplicate(of("1", "2", "3")); - - assertThat(String.join(", ", partners._1)).isEqualTo("1, 2, 3"); - assertThat(String.join(", ", partners._2)).isEqualTo("1, 2, 3"); - } - - @Test(timeout = 5_000L) // avoid endless test caused by infinite iterator - public void shouldDuplicateIteratorsLazily() { - final java.util.List itemsCalled = new java.util.ArrayList<>(); - - // Given an infinite iterator - final Iterator iterator = Iterator.iterate(1, i -> { - itemsCalled.add(i); - return i + 1; - }); - - // When duplicating it - final Tuple2, Iterator> partners = IteratorModule.duplicate(iterator); - - // Then the duplication is done lazily - assertThat(iterator.hasNext()).isTrue(); - assertThat(itemsCalled).as("Duplicating iterators should be done lazily.").isEmpty(); - - // When retrieving more elements - for (final int i : of(1, 2, 3)) { - assertThat(partners._1.hasNext()).isTrue(); - assertThat(partners._2.hasNext()).isTrue(); - assertThat(partners._1.next()).isEqualTo(i); - assertThat(partners._2.next()).isEqualTo(i); - } - - // Then the original iterator is called, once per next() - assertThat(itemsCalled).containsExactly(1, 2); - } - - @Test(timeout = 5_000L) // avoid endless test caused by infinite iterator - public void shouldLetBehindIteratorBecomeAheadIteratorWhenItMovesForward() { - final java.util.List itemsCalled = new java.util.ArrayList<>(); - - // Given an infinite iterator - final Iterator iterator = Iterator.iterate(1, i -> { - itemsCalled.add(i); - return i + 1; - }); - - // When duplicating it - final Tuple2, Iterator> partners = IteratorModule.duplicate(iterator); - // And move forward one iterator - assertThat(partners._1.hasNext()).isTrue(); - assertThat(partners._1.next()).isEqualTo(1); - // Then we have one iterator "ahead" and the other iterator "behind" - - // When moving forward the "behind" iterator - // Then it becomes the new iterator ahead because `duplicate()` should - // be able to change the ahead iterator - for (final int i : of(1, 2, 3, 4)) { - assertThat(partners._2.hasNext()).isTrue(); - assertThat(partners._2.next()).isEqualTo(i); - } - assertThat(itemsCalled).containsExactly(1, 2, 3); - } - // -- groupBy @Override @@ -803,7 +737,7 @@ public void shouldReturnSomeOnNextOptionWhenIteratorOfOneElement() { @Test public void shouldPartition() { - final Tuple2, Iterator> partitions = of("1", "2", "3").partition(i -> "2".equals(i)); + final Tuple2, Iterator> partitions = of("1", "2", "3").partition("2"::equals); assertThat(String.join(", ", partitions._1)).isEqualTo("2"); assertThat(String.join(", ", partitions._2)).isEqualTo("1, 3"); }