From 6c1494a04633402f833df31955e629f36de933c3 Mon Sep 17 00:00:00 2001 From: John DeRegnaucourt Date: Tue, 24 Dec 2024 15:07:47 -0500 Subject: [PATCH] Finally taking shape. More documentation coming, should be near final version. --- .../com/cedarsoftware/util/CompactMap.java | 320 +++++++++--------- .../cedarsoftware/util/CompactMapTest.java | 11 +- 2 files changed, 157 insertions(+), 174 deletions(-) diff --git a/src/main/java/com/cedarsoftware/util/CompactMap.java b/src/main/java/com/cedarsoftware/util/CompactMap.java index a7f9b6ac..5b3f2c8f 100644 --- a/src/main/java/com/cedarsoftware/util/CompactMap.java +++ b/src/main/java/com/cedarsoftware/util/CompactMap.java @@ -13,7 +13,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; -import java.lang.reflect.Method; import java.net.URI; import java.util.AbstractCollection; import java.util.AbstractMap; @@ -224,19 +223,17 @@ public CompactMap() { throw new IllegalStateException("compactSize() must be >= 2"); } - // Only check configuration for direct subclasses - if (getClass() != CompactMap.class && !getClass().getName().contains("caseSen_")) { - Method caseMethod = ReflectionUtils.getMethodAnyAccess(getClass(), "isCaseInsensitive", false); - boolean isOverridden = caseMethod != null && caseMethod.getDeclaringClass() != CompactMap.class; + // Only check direct subclasses, not our generated classes + if (getClass() != CompactMap.class && isLegacyConstructed()) { + Map map = getNewMap(); + if (map instanceof SortedMap) { + SortedMap sortedMap = (SortedMap)map; + Comparator comparator = sortedMap.comparator(); - if (isOverridden) { - Map map = getNewMap(); - if (map instanceof SortedMap) { - Comparator comparator = ((SortedMap)map).comparator(); - if (comparator == String.CASE_INSENSITIVE_ORDER && !isCaseInsensitive()) { - throw new IllegalStateException( - "Configuration mismatch: Map uses case-insensitive comparison but CompactMap is configured as case-sensitive"); - } + // Check case sensitivity consistency + if (comparator == String.CASE_INSENSITIVE_ORDER && !isCaseInsensitive()) { + throw new IllegalStateException( + "Inconsistent configuration: Map uses case-insensitive comparison but isCaseInsensitive() returns false"); } } } @@ -303,40 +300,12 @@ private boolean areKeysEqual(Object key, Object aKey) { /** * Compares two keys for ordering based on the map's ordering and case sensitivity settings. * - *

- * The comparison follows these rules: - *

    - *
  • If both keys are equal (as determined by {@link #areKeysEqual}), returns {@code 0}.
    • - *
  • If both keys are instances of {@link String}: - *
      - *
    • Uses a case-insensitive comparator if {@link #isCaseInsensitive()} is {@code true}; otherwise, uses case-sensitive comparison.
      • - *
    • Reverses the comparator if the map's ordering is set to {@code REVERSE}.
      • - *
    • If one keys is String and other is not, compares class names lexicographically to establish a consistent order (honoring {@code REVERSE} if needed).
      • - *
    - *
    • - *
  • If both keys implement {@link Comparable} and are of the exact same class: - *
      - *
    • Compares them using their natural ordering.
      • - *
    • Reverses the result if the map's ordering is set to {@code REVERSE}.
      • - *
    - *
    • - *
  • If keys are of different classes or do not implement {@link Comparable}: - *
      - *
    • Handles {@code null} values: {@code null} is considered less than any non-null key.
      • - *
    • Compares class names lexicographically to establish a consistent order (honoring {@code REVERSE} if needed)
      • - *
    - *
    • - *
- *

- * - *

Note: This method ensures a durable and consistent ordering, even for keys of differing types or non-comparable keys, by falling back to class name comparison.

- * * @param key1 the first key to compare * @param key2 the second key to compare - * @return a negative integer, zero, or a positive integer as {@code key1} is less than, equal to, - * or greater than {@code key2} + * @param forceReverse override to force reverse ordering regardless of map settings + * @return a negative integer, zero, or positive integer as key1 is less than, equal to, or greater than key2 */ - private int compareKeysForOrder(Object key1, Object key2) { + private int compareKeysForOrder(Object key1, Object key2, boolean forceReverse) { // 1. Handle nulls explicitly if (key1 == null) { return (key2 == null) ? 0 : 1; // Nulls last when sorting @@ -350,35 +319,50 @@ private int compareKeysForOrder(Object key1, Object key2) { return 0; } - // 3. Cache ordering and case sensitivity to avoid repeated method calls - String ordering = getOrdering(); - boolean isReverse = REVERSE.equals(ordering); - - // 4. String comparison - most common case - Class key1Class = key1.getClass(); - Class key2Class = key2.getClass(); - - if (key1Class == String.class) { - if (key2Class == String.class) { - int comparison = isCaseInsensitive() - ? String.CASE_INSENSITIVE_ORDER.compare((String) key1, (String) key2) - : ((String) key1).compareTo((String) key2); - return isReverse ? -comparison : comparison; + int result; + if (isLegacyConstructed()) { + if (isCaseInsensitive() && key1 instanceof String && key2 instanceof String) { + result = String.CASE_INSENSITIVE_ORDER.compare((String)key1, (String)key2); + } else if (key1 instanceof Comparable) { + result = ((Comparable)key1).compareTo(key2); + } else { + result = key1.getClass().getName().compareTo(key2.getClass().getName()); + } + } else { + // Non-legacy mode logic + Class key1Class = key1.getClass(); + Class key2Class = key2.getClass(); + + if (key1Class == String.class) { + if (key2Class == String.class) { + result = isCaseInsensitive() + ? String.CASE_INSENSITIVE_ORDER.compare((String) key1, (String) key2) + : ((String) key1).compareTo((String) key2); + } else { + // key1 is String, key2 is not - use class name comparison + result = key1Class.getName().compareTo(key2Class.getName()); + } + } else if (key1Class == key2Class && key1 instanceof Comparable) { + // Same type and comparable + result = ((Comparable) key1).compareTo(key2); + } else { + // Fallback to class name comparison for different types + result = key1Class.getName().compareTo(key2Class.getName()); } - // key1 is String, key2 is not - use class name comparison - int cmp = key1Class.getName().compareTo(key2Class.getName()); - return isReverse ? -cmp : cmp; } - // 5. Try Comparable if same type - if (key1Class == key2Class && key1 instanceof Comparable) { - int comparison = ((Comparable) key1).compareTo(key2); - return isReverse ? -comparison : comparison; - } + // Apply reverse ordering if needed + boolean shouldReverse = forceReverse || REVERSE.equals(getOrdering()); + return shouldReverse ? -result : result; + } - // 6. Fallback to class name comparison for different types - int cmp = key1Class.getName().compareTo(key2Class.getName()); - return isReverse ? -cmp : cmp; + // Remove the old two-parameter version and update all calls to use the three-parameter version + private int compareKeysForOrder(Object key1, Object key2) { + return compareKeysForOrder(key1, key2, false); + } + + private boolean isLegacyConstructed() { + return !getClass().getName().contains("caseSen_"); } /** @@ -584,61 +568,108 @@ private V removeFromCompactArray(Object key) { * @param array The array containing key-value pairs to sort */ private void sortCompactArray(final Object[] array) { + int pairCount = array.length / 2; + if (pairCount <= 1) { + return; + } + + if (isLegacyConstructed()) { + Map mapInstance = getNewMap(); // Called only once before iteration + boolean reverse = false; + + if (mapInstance instanceof SortedMap) { + SortedMap sortedMap = (SortedMap)mapInstance; + Comparator comparator = sortedMap.comparator(); + if (comparator != null) { + // Check if it's one of the reverse comparators from Collections + reverse = comparator.getClass().getName().toLowerCase().contains("reversecomp"); + } + + quickSort(array, 0, pairCount - 1, reverse); + } + return; + } + + // Non-legacy mode logic remains the same String ordering = getOrdering(); if (ordering.equals(UNORDERED) || ordering.equals(INSERTION)) { return; } - int pairCount = array.length / 2; - if (pairCount <= 1) { - return; + quickSort(array, 0, pairCount - 1, REVERSE.equals(ordering)); + } + + private void quickSort(Object[] array, int lowPair, int highPair, boolean reverse) { + if (lowPair < highPair) { + int pivotPair = partition(array, lowPair, highPair, reverse); + quickSort(array, lowPair, pivotPair - 1, reverse); + quickSort(array, pivotPair + 1, highPair, reverse); } + } - quickSort(array, 0, pairCount - 1); // Work with pair indices + private void swapPairs(Object[] array, int i, int j) { + Object tempKey = array[i]; + Object tempValue = array[i + 1]; + array[i] = array[j]; + array[i + 1] = array[j + 1]; + array[j] = tempKey; + array[j + 1] = tempValue; } - private void quickSort(Object[] array, int lowPair, int highPair) { - if (lowPair < highPair) { - int pivotPair = partition(array, lowPair, highPair); - quickSort(array, lowPair, pivotPair - 1); - quickSort(array, pivotPair + 1, highPair); + private Object selectPivot(Object[] array, int low, int mid, int high) { + Object first = array[low]; + Object middle = array[mid]; + Object last = array[high]; + + // Compare the three elements to find the median + if (compareKeysForOrder(first, middle, false) <= 0) { + if (compareKeysForOrder(middle, last, false) <= 0) { + swapPairs(array, mid, high); // median is middle + return middle; + } else if (compareKeysForOrder(first, last, false) <= 0) { + // median is last, already in position + return last; + } else { + swapPairs(array, low, high); // median is first + return first; + } + } else { + if (compareKeysForOrder(first, last, false) <= 0) { + swapPairs(array, low, high); // median is first + return first; + } else if (compareKeysForOrder(middle, last, false) <= 0) { + swapPairs(array, mid, high); // median is middle + return middle; + } else { + // median is last, already in position + return last; + } } } - private int partition(Object[] array, int lowPair, int highPair) { - // Convert pair indices to array indices + private int partition(Object[] array, int lowPair, int highPair, boolean reverse) { int low = lowPair * 2; int high = highPair * 2; + int mid = low + ((high - low) / 4) * 2; // Ensure we stay on key indices + + // Select pivot using median-of-three + Object pivot = selectPivot(array, low, mid, high); - // Use last element as pivot - K pivot = (K) array[high]; - int i = low - 2; // Start before first pair + int i = low - 2; for (int j = low; j < high; j += 2) { - if (compareKeysForOrder(array[j], pivot) <= 0) { + int comparison = compareKeysForOrder(array[j], pivot, reverse); + if (comparison <= 0) { i += 2; - // Swap pairs - Object tempKey = array[i]; - Object tempValue = array[i + 1]; - array[i] = array[j]; - array[i + 1] = array[j + 1]; - array[j] = tempKey; - array[j + 1] = tempValue; + swapPairs(array, i, j); } } - // Put pivot in correct position i += 2; - Object tempKey = array[i]; - Object tempValue = array[i + 1]; - array[i] = array[high]; - array[i + 1] = array[high + 1]; - array[high] = tempKey; - array[high + 1] = tempValue; - - return i / 2; // Return pair index + swapPairs(array, i, high); + return i / 2; } - + private void switchToMap(Object[] entries, K key, V value) { // Get the correct map type with initial capacity Map map = getNewMap(); // This respects subclass overrides @@ -1204,22 +1235,7 @@ protected int capacity() { } protected boolean isCaseInsensitive() { - // Skip inference for generated classes - if (getClass().getName().contains("caseSen_")) { - return false; - } - - // Do inference for direct subclasses - if (getClass() != CompactMap.class) { - Map map = getNewMap(); - if (map instanceof SortedMap) { - Comparator comparator = ((SortedMap)map).comparator(); - if (comparator == String.CASE_INSENSITIVE_ORDER) { - return true; - } - } - } - return false; + return !DEFAULT_CASE_SENSITIVE; } protected int compactSize() { @@ -1241,25 +1257,6 @@ protected int compactSize() { * @return the ordering strategy for this map */ protected String getOrdering() { - // Skip inference for generated classes - if (getClass().getName().contains("caseSen_")) { - return UNORDERED; - } - - // Do inference for direct subclasses - if (getClass() != CompactMap.class) { - Map map = getNewMap(); - if (map instanceof SortedMap) { - Comparator comparator = ((SortedMap)map).comparator(); - if (comparator != null) { - if (comparator.equals(Collections.reverseOrder(String.CASE_INSENSITIVE_ORDER)) || - comparator.equals(Collections.reverseOrder())) { - return REVERSE; - } - } - return SORTED; - } - } return UNORDERED; } @@ -1639,21 +1636,7 @@ private static Class determineMapType(Map options return rawMapType; } - - /** - * Returns {@code true} if this {@code CompactMap} instance is considered "legacy," - * meaning it is either: - *
    - *
  • A direct instance of CompactMap (not a subclass), or
    • - *
  • A subclass that does not override the {@code getOrdering()} method
    • - *
- * Returns {@code false} if it is a subclass that overrides {@code getOrdering()}. - */ - public boolean isLegacyCompactMap() { - return this.getClass() == CompactMap.class || - ReflectionUtils.getMethodAnyAccess(getClass(), "getOrdering", false) == null; - } - + /** * Creates a new CompactMapBuilder to construct a CompactMap with customizable properties. *

@@ -1794,31 +1777,25 @@ private static String generateSourceCode(String className, Map o String simpleClassName = className.substring(className.lastIndexOf('.') + 1); StringBuilder sb = new StringBuilder(); - // Package and imports + // Package declaration sb.append("package com.cedarsoftware.util;\n\n"); - sb.append("import java.util.*;\n\n"); - // Add import for the test class if needed + // Basic imports + sb.append("import java.util.*;\n"); + + // Add import for map type if it's in a different package Class mapType = (Class)options.get(MAP_TYPE); - if (mapType != null && mapType.getEnclosingClass() != null) { - sb.append("import ").append(mapType.getEnclosingClass().getName()).append(".*;\n"); + if (mapType != null && + !mapType.getPackage().getName().equals("com.cedarsoftware.util")) { + sb.append("import ").append(mapType.getName()).append(";\n"); } + sb.append("\n"); + // Class declaration sb.append("public class ").append(simpleClassName) .append(" extends CompactMap {\n"); - // Now explicitly override ALL configuration methods - appendAllConfigurationOverrides(sb, options); - - // Close class - sb.append("}\n"); - - String code = sb.toString(); - return code; - } - - private static void appendAllConfigurationOverrides(StringBuilder sb, Map options) { // Override isCaseInsensitive boolean caseSensitive = (boolean)options.getOrDefault(CASE_SENSITIVE, DEFAULT_CASE_SENSITIVE); sb.append(" @Override\n") @@ -1851,10 +1828,15 @@ private static void appendAllConfigurationOverrides(StringBuilder sb, Map options) { String ordering = (String)options.getOrDefault(ORDERING, UNORDERED); boolean caseSensitive = (boolean)options.getOrDefault(CASE_SENSITIVE, DEFAULT_CASE_SENSITIVE); diff --git a/src/test/java/com/cedarsoftware/util/CompactMapTest.java b/src/test/java/com/cedarsoftware/util/CompactMapTest.java index 35f61802..86ad7204 100644 --- a/src/test/java/com/cedarsoftware/util/CompactMapTest.java +++ b/src/test/java/com/cedarsoftware/util/CompactMapTest.java @@ -2750,7 +2750,7 @@ public void testCI() @Test public void testWrappedTreeMap() { - CompactMap m= new CompactMap() + CompactMap m = new CompactMap() { protected String getSingleValueKey() { return "a"; } protected Map getNewMap() { return new TreeMap<>(String.CASE_INSENSITIVE_ORDER); } @@ -2763,14 +2763,15 @@ public void testWrappedTreeMap() m.put("a", "alpha"); assert m.size() == 3; Iterator i = m.keySet().iterator(); - assert "a" == i.next(); + Object next = i.next(); + assert "a" == next; // Original failing assertion assert "J" == i.next(); assert "z" == i.next(); assert m.containsKey("A"); assert m.containsKey("j"); assert m.containsKey("Z"); } - + @Test public void testMultipleSortedKeysetIterators() { @@ -3498,8 +3499,8 @@ public void testPerformance() { int maxSize = 1000; final int[] compactSize = new int[1]; - int lower = 40; - int upper = 120; + int lower = 50; + int upper = 100; long totals[] = new long[upper - lower + 1]; for (int x = 0; x < 2000; x++)