review

Author: triceo

core/src/main/java/ai/timefold/solver/core/impl/util/ElementAwareArrayList.java

@@ -86,7 +86,8 @@ private Entry getEntry(int index) {
         } else if (gapCount == 0 || index < firstGapPosition) {
             return (Entry) entries[index];
         }
-        return partialCompact(index);
+        partialCompact(index);
+        return (Entry) entries[index];
     }
 
     /**
@@ -103,10 +104,10 @@ void compact() {
     /**
      * Avoid calling this when {@code gapCount == 0}.
      */
-    private Entry partialCompact(int rightBoundaryPosition) {
+    private void partialCompact(int rightBoundaryPosition) {
         if (rightBoundaryPosition < firstGapPosition) {
             // The entire target range is in the already-compacted prefix; no work needed.
-            return (Entry) entries[rightBoundaryPosition];
+            return;
         }
         var encounteredGaps = 0;
         var lastNonNullPosition = firstGapPosition - 1; // firstGapPosition non-nulls are already in place before us.
@@ -131,7 +132,7 @@ private Entry partialCompact(int rightBoundaryPosition) {
                     } else {
                         firstGapPosition = rightBoundaryPosition + 1;
                     }
-                    return entry;
+                    return;
                 }
             }
         }
@@ -174,7 +175,7 @@ public void add(int index, T element) {
         }
         // Compact prefix [0, index-1] so physical position k == logical position k for all k < index.
         if (index > 0) {
-            partialCompact(index - 1); // Increases modCount.
+            partialCompact(index - 1);
         }
         if (entries[index] == null) {
             // Gap at the target position: fill it directly without shifting the array.
@@ -385,7 +386,9 @@ private ElementAwareListIterator(int startingPosition) {
                         "The index (%d) must be >= 0 and <= size (%d).".formatted(startingPosition, currentSize));
             }
             if (startingPosition > 0 && gapCount > 0) {
-                currentPosition = partialCompact(startingPosition - 1).position + 1;
+                var index = startingPosition - 1;
+                partialCompact(index);
+                currentPosition = ((Entry) entries[index]).position + 1;
             } else {
                 currentPosition = startingPosition;
             }

core/src/test/java/ai/timefold/solver/core/impl/util/ElementAwareArrayListTest.java

@@ -125,21 +125,6 @@ void addEntryAfterChurnAppendsInOrder() {
             assertThat(entryX.toString()).contains("@1");
         }
 
-        @Test
-        @DisplayName("remove-to-empty frees large backing array (length > RETAIN_THRESHOLD)")
-        void removeToEmptyFreesLargeBackingArray() {
-            var list = new ElementAwareArrayList<String>();
-            // 30 elements force entries.length to 32 (> RETAIN_THRESHOLD=16), triggering the free path on empty.
-            List<ElementAwareArrayList<String>.Entry> entryList = new ArrayList<>();
-            for (var i = 0; i < 30; i++) {
-                entryList.add(list.addEntry("e" + i));
-            }
-            for (var entry : entryList) {
-                entry.remove();
-            }
-            assertThat(list).isEmpty();
-        }
-
         @Test
         @DisplayName("add after free-path empty re-allocates and preserves insertion order")
         void addAfterLargeArrayFreePathPreservesOrder() {
@@ -1520,8 +1505,11 @@ void stressTestAgainstReferenceArrayList(long seed) {
                         list.compact();
                         assertThat(list).containsExactlyElementsOf(reference);
                     }
+                    default -> {
+                        throw new IllegalStateException("Unexpected operation code: %d".formatted(op));
+                    }
                 }
-                assertThat(list.size()).isEqualTo(reference.size());
+                assertThat(list).hasSameSizeAs(reference);
             }
             assertThat(list).containsExactlyElementsOf(reference);
         }