Fixed Bug #47: 'RrbTree.split() returns wrong type' and added test to…

… prevent regressions. Thank you, @fcurts, for reporting this

CHANGE_LOG.md

@@ -5,6 +5,10 @@ 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.
 
+## Release 3.7.2 2021-12-20: Fix RrbTree.split()
+- Fixed Bug #47: "RrbTree.split() returns wrong type" and added test to prevent regressions.
+  Thank you, @fcurts, for reporting this!
+
 ### Release 3.7.1 2021-12-20
 - Added BaseList.appendWhen() for fluent list building with optional additions.
 

README.md

@@ -65,11 +65,8 @@ If you work with pure Java, or a mix of Java and Kotlin files, this is your bran
 If you want to live dangerously, try the all-Kotlin version in the 4.0 branch when it becomes available.
 
 # News
-## Don't use RrbTree.join()!
-@jafingerhut has found bugs in the RRB Tree join implementation.
-See [issue 31](https://github.com/GlenKPeterson/Paguro/issues/31) and [36](https://github.com/GlenKPeterson/Paguro/issues/36).
-I normally try to fix bugs promptly, this issue requires more thought!
-If anyone knows of a paper with a working algorithm for merging two n-ary BTrees, let us know!
+## RrbTree.join() seems to work now
+RrbTree is still a new class, but as of 3.7.2, there are no known bugs.  Fingers crossed!
 
 Check the [Change Log](CHANGE_LOG.md) for details of recent changes.
 

pom.xml

@@ -38,7 +38,7 @@ http://mvnrepository.com/artifact/org.organicdesign/Paguro
 	-->
 	<groupId>org.organicdesign</groupId>
 	<artifactId>Paguro</artifactId>
-	<version>3.7.1</version>
+	<version>3.7.2</version>
 	<packaging>jar</packaging>
 
 	<name>Paguro</name>

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

@@ -561,7 +561,7 @@ void debugValidate() {
         @Override public int size() { return size; }
 
         @Override
-        protected @NotNull MutRrbt<E> mt() { return emptyMutable(); }
+        protected @NotNull ImRrbt<E> mt() { return empty(); }
 
         /**
          Divides this RRB-Tree such that every index less-than the given index ends up in the left-hand
@@ -856,6 +856,7 @@ assume that I know (I don't), or if someone has come up with a better way to do
      */
     protected abstract @NotNull RrbTree<E> makeNew(E @NotNull [] f, int fi, int fl, @NotNull Node<E> r, int s);
 
+    /** Creates a new empty ("M-T") tree of the appropriate (mutable/immutable) type. */
     protected abstract @NotNull RrbTree<E> mt();
 
     /** Internal method - do not use. */

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

@@ -13,6 +13,7 @@
 // limitations under the License.
 package org.organicdesign.fp.collections;
 
+import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 import org.organicdesign.fp.StaticImports;
 import org.organicdesign.fp.TestUtilities;
@@ -516,7 +517,12 @@ private static boolean isPrime(int num) {
 //        return true;
     }
 
-    private static <T> void testSplit(ArrayList<T> control, RrbTree<T> test, int splitIndex) {
+    private static <T> void testSplit(
+            @SuppressWarnings("rawtypes") @NotNull Class<? extends RrbTree> expectedClass,
+            @NotNull ArrayList<T> control,
+            @NotNull RrbTree<T> test,
+            int splitIndex
+    ) {
         if ( (splitIndex < 0) || (splitIndex > control.size()) ) {
             throw new IllegalArgumentException("Constraint violation failed: 1 <= splitIndex <= size");
         }
@@ -527,7 +533,15 @@ private static <T> void testSplit(ArrayList<T> control, RrbTree<T> test, int spl
         List<T> leftControl = control.subList(0, splitIndex);
         List<T> rightControl = control.subList(splitIndex, control.size());
         RrbTree<T> leftSplit = split._1();
+        if (!expectedClass.isAssignableFrom(leftSplit.getClass())) {
+            fail("splitIndex " + splitIndex + " left " + leftSplit.getClass() +
+                    " not an instance of " + expectedClass);
+        }
         RrbTree<T> rightSplit = split._2();
+        if (!expectedClass.isAssignableFrom(rightSplit.getClass())) {
+            fail("splitIndex " + splitIndex + " right " + rightSplit.getClass() +
+                    " not an instance of " + expectedClass);
+        }
         if (isPrime(splitIndex)) {
             System.out.println("original=\n" + test.indentedStr(0));
             System.out.println("splitIndex=" + splitIndex);
@@ -550,7 +564,7 @@ private static <T> void testSplit(ArrayList<T> control, RrbTree<T> test, int spl
             is = is.append(i);
             control.add(i);
         }
-        testSplit(control, is, 29);
+        testSplit(ImRrbt.class, control, is, 29);
     }
 
     @Test public void strictSplitTest() {
@@ -570,12 +584,12 @@ private static <T> void testSplit(ArrayList<T> control, RrbTree<T> test, int spl
 //            int splitIndex = i; //rand.nextInt(is.size() + 1);
 //            System.out.println("splitIndex=" + splitIndex);
 //        System.out.println("empty=" + RrbTree.empty().indentedStr(6));
-                testSplit(control, is, splitIndex);
-                testSplit(control, ms, splitIndex);
+                testSplit(ImRrbt.class, control, is, splitIndex);
+                testSplit(MutRrbt.class, control, ms, splitIndex);
             }
             splitIndex = TWO_LEVEL_SZ;
-            testSplit(control, is, splitIndex);
-            testSplit(control, ms, splitIndex);
+            testSplit(ImRrbt.class, control, is, splitIndex);
+            testSplit(MutRrbt.class, control, ms, splitIndex);
         } catch (Exception e) {
             System.out.println("Bad splitIndex (im): " + splitIndex); // print before blowing up...
             System.out.println("before split (im): " + is.indentedStr(13)); // print before blowing up...
@@ -603,12 +617,12 @@ private static <T> void testSplit(ArrayList<T> control, RrbTree<T> test, int spl
 //            System.out.println("is:" + is.indentedStr(3));
             for (int j = 0; j <= ONE_LEVEL_SZ; j+= ONE_LEVEL_SZ / 10) {
                 splitIndex = j; // So we have it when exception is thrown.
-                testSplit(control, is, splitIndex);
-                testSplit(control, ms, splitIndex);
+                testSplit(ImRrbt.class, control, is, splitIndex);
+                testSplit(MutRrbt.class, control, ms, splitIndex);
             }
             splitIndex = ONE_LEVEL_SZ;
-            testSplit(control, is, splitIndex);
-            testSplit(control, ms, splitIndex);
+            testSplit(ImRrbt.class, control, is, splitIndex);
+            testSplit(MutRrbt.class, control, ms, splitIndex);
         } catch (Exception e) {
             System.out.println("splitIndex:" + splitIndex + " rands:" + rands); // print before blowing up...
             // OK, now we can continue throwing exception.
@@ -936,7 +950,7 @@ private static <T> RrbTree<T> mut(T... ts) {
     }
 
     /**
-     * New test submitted by J.A. Fingerhut
+     * Test submitted by J.A. Fingerhut
      */
     @Test public void joinMutableTest2() {
         int b = STRICT_NODE_LENGTH;
@@ -969,7 +983,7 @@ private static <T> RrbTree<T> mut(T... ts) {
     }
 
     /**
-     * New test submitted by J.A. Fingerhut
+     * Test submitted by J.A. Fingerhut
      */
     @Test public void joinImTest2() {
         int b = STRICT_NODE_LENGTH;
@@ -1000,7 +1014,6 @@ private static <T> RrbTree<T> mut(T... ts) {
         }
     }
 
-    @SuppressWarnings("deprecation")
     @Test public void testBiggerJoin() {
         ImRrbt<Integer> is = RrbTree.empty();
         MutRrbt<Integer> ms = RrbTree.emptyMutable();
@@ -1019,8 +1032,6 @@ private static <T> RrbTree<T> mut(T... ts) {
         }
     }
 
-
-    @SuppressWarnings("deprecation")
     @Test public void testWithout() {
         assertEquals(rrb(1,2,3,5,6), rrb(1,2,3,4,5,6).without(3));
         assertEquals(mut(1,2,3,5,6), mut(1,2,3,4,5,6).without(3));