- Implemented concat() and precat() methods on RrbTree and precat() …

…on ImRrbTree and MutRrbTree

   because:
   - concat() was implemented on PersistentVector() because it's a cheap operation.
   - concat() and precat() are cheap and sensible on the RRB Tree.
 - Added a few @NotNull and @contract annotations where they were missing.

CHANGE_LOG.md

@@ -5,6 +5,25 @@ releases on the way from an old version to a new one.  Fix any deprecation warni
 release before upgrading to the next one.  The documentation next to each Deprecated annotation
 tells you what to use instead.  Once we delete the deprecated methods, that documentation goes too.
 
+// CONSIDER public interface UnmodMap<K,V> extends Map<K,V>, UnmodIterable<UnmodMap.UnEntry<K,V>>, Sized {
+// CONSIDER Changing to Map.Entry
+## Release 3.10.3 2022-05-08: concat() and precat()
+ - Implemented concat() and precat() methods on RrbTree and precat() on ImRrbTree and MutRrbTree
+   because:
+   - concat() was implemented on PersistentVector() because it's a cheap operation.
+   - concat() and precat() are cheap and sensible on the RRB Tree.
+ - Added a few @NotNull and @Contract annotations where they were missing.
+
+### Release 3.10.2 2022-05-08: Oops!
+Thought I could implement concat and precat on unsorted maps, but no.  Here's why:
+
+SortedSet(3, 4, 5).precat(1, 2) yields an Xform that starts with 1, 2, then however the source is iterated (3, 4, 5).
+So it insures that the precat-stuff comes first.
+Similarly SortedSet(3, 4, 5).concat(6, 7) yields the itrerated source, then the to-be-concatenated additions.
+I just made UnorderedSet(3, 4, 5).precat(6, 7) return a set containing the numbers 3-7 in any order.
+That's not what precat (or concat) is supposed to do.
+I'm going to undo this!
+
 ## Release 3.10.1 2022-03-18: NotNull whereNotNull()
  - Added `@NotNull` annotation to the generic type parameter returned by Transformable.whereNonNull()
    (and implementation of that method in UnmodIterable).

Paguro.iml

@@ -10,8 +10,10 @@
     </content>
     <orderEntry type="inheritedJdk" />
     <orderEntry type="sourceFolder" forTests="false" />
-    <orderEntry type="library" scope="TEST" name="Maven: junit:junit:4.13.2" level="project" />
-    <orderEntry type="library" scope="TEST" name="Maven: org.hamcrest:hamcrest-core:1.3" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: org.junit.jupiter:junit-jupiter-api:5.8.2" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: org.opentest4j:opentest4j:1.2.0" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: org.junit.platform:junit-platform-commons:1.8.2" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: org.apiguardian:apiguardian-api:1.1.2" level="project" />
     <orderEntry type="library" name="Maven: org.jetbrains:annotations:23.0.0" level="project" />
     <orderEntry type="library" scope="TEST" name="Maven: org.organicdesign:TestUtils:1.0.6" level="project" />
     <orderEntry type="library" scope="TEST" name="Maven: org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.10" level="project" />
@@ -20,6 +22,8 @@
     <orderEntry type="library" scope="TEST" name="Maven: org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.6.10" level="project" />
     <orderEntry type="library" scope="TEST" name="Maven: org.organicdesign:Indented:0.0.20" level="project" />
     <orderEntry type="library" scope="TEST" name="Maven: javax.servlet:javax.servlet-api:4.0.1" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: junit:junit:4.13.2" level="project" />
+    <orderEntry type="library" scope="TEST" name="Maven: org.hamcrest:hamcrest-core:1.3" level="project" />
     <orderEntry type="library" scope="TEST" name="Maven: org.jetbrains.kotlin:kotlin-test-junit:1.6.10" level="project" />
     <orderEntry type="library" scope="TEST" name="Maven: org.jetbrains.kotlin:kotlin-test:1.6.10" level="project" />
   </component>

pom.xml

@@ -38,7 +38,7 @@ http://mvnrepository.com/artifact/org.organicdesign/Paguro
 	-->
 	<groupId>org.organicdesign</groupId>
 	<artifactId>Paguro</artifactId>
-	<version>3.10.1</version>
+	<version>3.10.3</version>
 	<packaging>jar</packaging>
 
 	<name>Paguro</name>
@@ -129,7 +129,7 @@ http://mvnrepository.com/artifact/org.organicdesign/Paguro
 				<groupId>org.apache.maven.plugins</groupId>
 				<artifactId>maven-javadoc-plugin</artifactId>
 				<!-- JavaDoc wasn't providing the right links to the Java APIs with the default version of the plugin on my machine. -->
-				<version>3.3.1</version>
+				<version>3.4.0</version>
 				<configuration>
 					<additionalOptions>-Xdoclint:none</additionalOptions>
 				</configuration>
@@ -145,7 +145,7 @@ http://mvnrepository.com/artifact/org.organicdesign/Paguro
 			<plugin>
 				<groupId>org.jacoco</groupId>
 				<artifactId>jacoco-maven-plugin</artifactId>
-				<version>0.8.7</version>
+				<version>0.8.8</version>
 				<executions>
 					<execution>
 						<goals>
@@ -164,7 +164,7 @@ http://mvnrepository.com/artifact/org.organicdesign/Paguro
 			<plugin>
 				<groupId>org.sonatype.plugins</groupId>
 				<artifactId>nexus-staging-maven-plugin</artifactId>
-				<version>1.6.8</version>
+				<version>1.6.13</version>
 				<extensions>true</extensions>
 				<configuration>
 					<serverId>ossrh</serverId>
@@ -176,9 +176,9 @@ http://mvnrepository.com/artifact/org.organicdesign/Paguro
 	</build>
 	<dependencies>
 		<dependency>
-			<groupId>junit</groupId>
-			<artifactId>junit</artifactId>
-			<version>4.13.2</version>
+			<groupId>org.junit.jupiter</groupId>
+			<artifactId>junit-jupiter-api</artifactId>
+			<version>5.8.2</version>
 			<scope>test</scope>
 		</dependency>
 		<dependency>

src/main/java/org/organicdesign/fp/collections/ImSet.java

@@ -13,14 +13,15 @@
 // limitations under the License.
 package org.organicdesign.fp.collections;
 
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 /** An immutable set with no guarantees about its ordering */
 public interface ImSet<E> extends BaseSet<E> {
 
     /** Returns a mutable version of this immutable set. */
-    MutSet<E> mutable();
+    @NotNull MutSet<E> mutable();
 
     /**
      Adds an element, returning a modified version of the set (leaving the original set unchanged).
@@ -34,11 +35,10 @@ element is the same as an old element (based on the address of that item in memo
     @NotNull
     @Override ImSet<E> put(E e);
 
+    @Contract(pure = true)
     default @NotNull ImSet<E> union(@Nullable Iterable<? extends E> iter) {
-        if (iter == null) { return this; }
-        ImSet<E> ret = this;
-        for (E e : iter) { ret = ret.put(e); }
-        return ret;
+        return iter == null ? this
+                            : mutable().union(iter).immutable();
     }
 
     /** {@inheritDoc} */

src/main/java/org/organicdesign/fp/collections/MutSet.java

@@ -14,6 +14,7 @@
 
 package org.organicdesign.fp.collections;
 
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 
 /**
@@ -30,9 +31,15 @@ public interface MutSet<E> extends BaseSet<E> {
     MutSet<E> put(E val);
 
     /** {@inheritDoc} */
-    @NotNull
-    @Override default MutSet<E> union(Iterable<? extends E> iter) {
-        return concat(iter).toMutSet();
+    @Override
+    @Contract(mutates = "this")
+    default @NotNull MutSet<E> union(Iterable<? extends E> iter) {
+        if (iter != null) {
+            for (E item : iter) {
+                put(item);
+            }
+        }
+        return this;
     }
 
     /** {@inheritDoc} */

src/main/java/org/organicdesign/fp/collections/PersistentHashMap.java

@@ -420,8 +420,9 @@ private MutHashMap(Equator<K> e, AtomicReference<Thread> edit, INode<K,V> root,
 
         @Override public Equator<K> equator() { return equator; }
 
-        @NotNull
-        @Override public MutHashMap<K,V> assoc(K key, V val) {
+        @Override
+        @Contract(mutates = "this")
+        public @NotNull MutHashMap<K,V> assoc(K key, V val) {
             ensureEditable();
             if (key == null) {
                 if (this.nullValue != val)

src/main/java/org/organicdesign/fp/collections/PersistentHashSet.java

@@ -20,6 +20,7 @@
 import java.io.Serializable;
 import java.util.Map;
 
+import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
 
 /**
@@ -120,7 +121,7 @@ private void writeObject(ObjectOutputStream s) throws IOException {
         @SuppressWarnings("unchecked")
         private void readObject(ObjectInputStream s) throws IOException, ClassNotFoundException {
             s.defaultReadObject();
-            MutMap tempMap = PersistentHashMap.<K,K>empty().mutable();
+            MutMap<K,K> tempMap = PersistentHashMap.<K,K>empty().mutable();
             for (int i = 0; i < size; i++) {
                 K k = (K) s.readObject();
                 tempMap = tempMap.assoc(k, k);
@@ -168,7 +169,8 @@ private void readObject(java.io.ObjectInputStream in) throws IOException,
 
     @Override public int size() { return impl.size(); }
 
-    public MutHashSet<E> mutable() {
+    @Contract(pure = true)
+    public @NotNull MutHashSet<E> mutable() {
         return new MutHashSet<>(impl.mutable());
     }
 
@@ -204,7 +206,9 @@ public static final class MutHashSet<E> extends AbstractUnmodSet<E>
             return this;
         }
 
-        @Override  public PersistentHashSet<E> immutable() {
+        @Override
+        @Contract(pure = true)
+        public @NotNull PersistentHashSet<E> immutable() {
             return new PersistentHashSet<>(impl.immutable());
         }
     }

src/main/java/org/organicdesign/fp/collections/RrbTree.java

@@ -143,6 +143,26 @@ public static class MutRrbt<E> extends RrbTree<E> implements MutList<E> {
             return (MutRrbt<E>) MutList.super.concat(es);
         }
 
+        /** {@inheritDoc} */
+        @Override
+        @Contract(mutates = "this")
+        public @NotNull MutRrbt<E> precat(@Nullable Iterable<? extends E> es) {
+            // We could check if es is sized and has a size >= MIN_NODE_LENGTH
+            // and if so, do something clever with writing chunks to the underlying tree.
+            // Until then, this will handle everything almost as well.
+            //
+            // If we insert(0, item) in order, they will end up reversed, so we have to
+            // add one to the insertion point for each item added.
+            int idx = 0;
+            if (es != null) {
+                for (E e : es) {
+                    insert(idx, e);
+                    idx++;
+                }
+            }
+            return this;
+        }
+
         void debugValidate() {
             if (focusLength > STRICT_NODE_LENGTH) {
                 throw new IllegalStateException("focus len:" + focusLength +
@@ -441,10 +461,18 @@ private void readObject(java.io.ObjectInputStream in) throws IOException,
 
         /** {@inheritDoc} */
         @Override
+        @Contract(pure = true)
         public @NotNull ImRrbt<E> concat(@Nullable Iterable<? extends E> es) {
             return this.mutable().concat(es).immutable();
         }
 
+        /** {@inheritDoc} */
+        @Override
+        @Contract(pure = true)
+        public @NotNull ImRrbt<E> precat(@Nullable Iterable<? extends E> es) {
+            return this.mutable().precat(es).immutable();
+        }
+
         void debugValidate() {
             if (focus.length > STRICT_NODE_LENGTH) {
                 throw new IllegalStateException("focus len:" + focus.length +
@@ -870,6 +898,14 @@ assume that I know (I don't), or if someone has come up with a better way to do
     /** Creates a new empty ("M-T") tree of the appropriate (mutable/immutable) type. */
     protected abstract @NotNull RrbTree<E> mt();
 
+    /**
+     * {@inheritDoc}
+     * Precat is implemented here because it is a very cheap operation on an RRB-Tree.
+     * It's not implemented on PersistentVector because it is very expensive there.
+     */
+    @Override
+    public abstract @NotNull RrbTree<E> precat(@Nullable Iterable<? extends E> es);
+
     /** Internal method - do not use. */
     abstract @NotNull Node<E> pushFocus();
 

src/test/java/org/organicdesign/fp/collections/ImMapTest.java

@@ -1,27 +1,18 @@
 package org.organicdesign.fp.collections;
 
 import org.jetbrains.annotations.NotNull;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 import org.organicdesign.fp.oneOf.Option;
 import org.organicdesign.fp.tuple.Tuple2;
 
 import java.util.HashMap;
 import java.util.Map;
 
-import static org.junit.Assert.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.organicdesign.fp.FunctionUtils.ordinal;
 
-/**
- Created by gpeterso on 9/13/16.
- */
 public class ImMapTest {
     private static class TestMap<K,V> implements ImMap<K,V> {
-        private static <K,V> Map<K,V> copyMap(Map<K,V> m) {
-            Map<K,V> ret = new HashMap<>();
-            ret.putAll(m);
-            return ret;
-        }
-
         private final Map<K,V> inner;
 
         private TestMap(Map<K,V> m) { inner = m; }
@@ -32,9 +23,9 @@ private static <K,V> Map<K,V> copyMap(Map<K,V> m) {
                                           : Option.none();
         }
 
-        @NotNull
-        @Override public ImMap<K, V> assoc(K key, V val) {
-            Map<K,V> m = copyMap(inner);
+        @Override
+        public @NotNull ImMap<K, V> assoc(K key, V val) {
+            Map<K,V> m = new HashMap<>(inner);
             m.put(key, val);
             return new TestMap<>(m);
         }
@@ -48,18 +39,18 @@ private static <K,V> Map<K,V> copyMap(Map<K,V> m) {
             throw new UnsupportedOperationException("not implemented");
         }
 
-        @NotNull
-        @Override public ImMap<K, V> without(K key) {
-            Map<K,V> m = copyMap(inner);
+        @Override
+        public @NotNull ImMap<K, V> without(K key) {
+            Map<K,V> m = new HashMap<>(inner);
             m.remove(key);
             return new TestMap<>(m);
         }
 
         @Override public int size() { return inner.size(); }
 
-        @NotNull
-        @Override public ImSet<K> keySet() {
-            return new ImSetTest.TestSet<>(inner.keySet());
+        @Override
+        public @NotNull ImSet<K> keySet() {
+            return new ImSetTest.ImTestSet<>(inner.keySet());
         }
 
         @NotNull
@@ -68,7 +59,8 @@ private static <K,V> Map<K,V> copyMap(Map<K,V> m) {
         }
     }
 
-    @Test public void testAssoc() {
+    @Test
+    public void testAssoc() {
         Map<String,Integer> control = new HashMap<>();
         ImMap<String,Integer> test = new TestMap<>(new HashMap<>());
         for (int i = 0; i < 20; i++) {