From 362838c434cd3115d5e8b92b07fb1310948b93d7 Mon Sep 17 00:00:00 2001 From: Speiger Date: Fri, 10 Dec 2021 05:54:37 +0100 Subject: [PATCH] Found something in FastUtil i needed to address too. BulkFixes coming. - Added: Guava TestSuit - Fixed: HashCode and toString method would crash if the Object Key/Value was null - Added: AbstractTypeCollection now delegates the contains check to type-specific Collections if it detects it. - Fixed: Map.Entry toString wasn't writing values not like it should do. - Fixed: Set.hashCode now is the sum of the elements instead of a Unique HashCode based on the elements. - Fixed: Added missing NonNull Checks. - Fixed: OpenHashMap.containsValue implementation was wrong. - Fixed: OpenHashMap.compute/present/absent now works how it is specified in the Java Documentation - Fixed: OpenHashMap.merge/BulkMerge now works how it is specified in the Java Documentation - Fixed: OpenHashMap.keySet.remove was causing a infinite loop. - Fixed: OpenHashMap.mapIterator now no longer crashes in certain cases. --- Changelog.md | 11 ++++ build.gradle | 2 + .../speiger/src/builder/GlobalVariables.java | 4 +- .../collections/AbstractCollection.template | 6 +++ .../maps/abstracts/AbstractMap.template | 2 +- .../maps/impl/hash/OpenHashMap.template | 50 +++++++++++++------ .../templates/maps/interfaces/Map.template | 11 +++- .../templates/sets/AbstractSet.template | 29 +++-------- .../src/collections/objects/map/MapTest.java | 40 +++++++++++++++ 9 files changed, 114 insertions(+), 41 deletions(-) create mode 100644 src/test/java/speiger/src/collections/objects/map/MapTest.java diff --git a/Changelog.md b/Changelog.md index 47f40b81..0b70f398 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,17 @@ - Fixed: EnumMaps didn't keep track of their size and now got proper care and implementations as needed. There might be more work required but at least the core functionality is now up to date. - Added: Tests for the new Stream replace functions to ensure no bugs are left. - Fixed: Custom HashSet reduce function with a default value was checking incorrectly for present keys. +- Added: Guava TestSuit +- Fixed: HashCode and toString method would crash if the Object Key/Value was null +- Added: AbstractTypeCollection now delegates the contains check to type-specific Collections if it detects it. +- Fixed: Map.Entry toString wasn't writing values not like it should do. +- Fixed: Set.hashCode now is the sum of the elements instead of a Unique HashCode based on the elements. +- Fixed: Added missing NonNull Checks. +- Fixed: OpenHashMap.containsValue implementation was wrong. +- Fixed: OpenHashMap.compute/present/absent now works how it is specified in the Java Documentation +- Fixed: OpenHashMap.merge/BulkMerge now works how it is specified in the Java Documentation +- Fixed: OpenHashMap.keySet.remove was causing a infinite loop. +- Fixed: OpenHashMap.mapIterator now no longer crashes in certain cases. ### Version 0.4.5 - Added: removeAll/retainAll(Collection c, Consumer r) which receives all the elements that got deleted from the collection diff --git a/build.gradle b/build.gradle index 2e2cb66e..9d54745f 100644 --- a/build.gradle +++ b/build.gradle @@ -45,6 +45,8 @@ dependencies { builderCompile 'de.speiger:Simple-Code-Generator:1.0.5' runtimeOnly 'de.speiger:Simple-Code-Generator:1.0.5' testImplementation 'junit:junit:4.12' + testImplementation 'com.google.guava:guava-testlib:31.0.1-jre' + } task generateSource(type: JavaExec) { diff --git a/src/builder/java/speiger/src/builder/GlobalVariables.java b/src/builder/java/speiger/src/builder/GlobalVariables.java index c230d6fd..192d2283 100644 --- a/src/builder/java/speiger/src/builder/GlobalVariables.java +++ b/src/builder/java/speiger/src/builder/GlobalVariables.java @@ -128,8 +128,8 @@ public class GlobalVariables addInjectMapper("OBJ_TO_"+fix, type.isObject() ? "%s" : "%s."+type.getKeyType(value)+"Value()").removeBraces(); addInjectMapper("CLASS_TO_"+fix, type.isObject() ? "("+type.getKeyType(value)+")%s" : "(("+type.getClassType(value)+")%s)."+type.getKeyType(value)+"Value()").removeBraces(); - addInjectMapper(fix+"_TO_HASH", type.isObject() ? "%s.hashCode()" : type.getClassType(value)+".hashCode(%s)").removeBraces(); - addInjectMapper(fix+"_TO_STRING", type.isObject() ? "%s.toString()" : type.getClassType(value)+".toString(%s)").removeBraces(); + addInjectMapper(fix+"_TO_HASH", type.isObject() ? "Objects.hashCode(%s)" : type.getClassType(value)+".hashCode(%s)").removeBraces(); + addInjectMapper(fix+"_TO_STRING", type.isObject() ? "Objects.toString(%s)" : type.getClassType(value)+".toString(%s)").removeBraces(); addSimpleMapper("CAST_"+fix+"_ARRAY ", type.isObject() ? "("+fix+"_TYPE[])" : ""); addSimpleMapper("EMPTY_"+fix+"_ARRAY", type.isObject() ? "("+fix+"_TYPE[])ARRAYS.EMPTY_ARRAY" : "ARRAYS.EMPTY_ARRAY"); diff --git a/src/builder/resources/speiger/assets/collections/templates/collections/AbstractCollection.template b/src/builder/resources/speiger/assets/collections/templates/collections/AbstractCollection.template index 1ec6f1f7..083ad9af 100644 --- a/src/builder/resources/speiger/assets/collections/templates/collections/AbstractCollection.template +++ b/src/builder/resources/speiger/assets/collections/templates/collections/AbstractCollection.template @@ -90,6 +90,12 @@ public abstract class ABSTRACT_COLLECTION KEY_GENERIC_TYPE extends AbstractColle return true; } + @Override + public boolean containsAll(Collection c) { + Objects.requireNonNull(c); + return c instanceof COLLECTION ? containsAll((COLLECTION KEY_GENERIC_TYPE)c) : super.containsAll(c); + } + /** * This implementation iterates over the elements of the collection and checks if they are stored in this collection * @param c the elements that should be checked for diff --git a/src/builder/resources/speiger/assets/collections/templates/maps/abstracts/AbstractMap.template b/src/builder/resources/speiger/assets/collections/templates/maps/abstracts/AbstractMap.template index b8a4c65c..ac4290c2 100644 --- a/src/builder/resources/speiger/assets/collections/templates/maps/abstracts/AbstractMap.template +++ b/src/builder/resources/speiger/assets/collections/templates/maps/abstracts/AbstractMap.template @@ -504,7 +504,7 @@ public abstract class ABSTRACT_MAP KEY_VALUE_GENERIC_TYPE extends AbstractMap" + VALUE_TO_STRING(value); + return KEY_TO_STRING(key) + "=" + VALUE_TO_STRING(value); } } } \ No newline at end of file diff --git a/src/builder/resources/speiger/assets/collections/templates/maps/impl/hash/OpenHashMap.template b/src/builder/resources/speiger/assets/collections/templates/maps/impl/hash/OpenHashMap.template index e0c26cec..2c078c77 100644 --- a/src/builder/resources/speiger/assets/collections/templates/maps/impl/hash/OpenHashMap.template +++ b/src/builder/resources/speiger/assets/collections/templates/maps/impl/hash/OpenHashMap.template @@ -278,8 +278,7 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE #if !VALUE_OBJECT @Override public boolean containsValue(VALUE_TYPE value) { - if(VALUE_EQUALS(value, values[nullIndex])) return true; - for(int i = nullIndex-1;i >= 0;i--) + for(int i = nullIndex;i >= 0;i--) if(KEY_EQUALS_NOT_NULL(keys[i]) && VALUE_EQUALS(values[i], value)) return true; return false; } @@ -288,9 +287,12 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE @Override @ValuePrimitive public boolean containsValue(Object value) { - if((value == null && VALUE_EQUALS(values[nullIndex], getDefaultReturnValue())) || EQUALS_VALUE_TYPE(values[nullIndex], value)) return true; - for(int i = nullIndex-1;i >= 0;i--) + for(int i = nullIndex;i >= 0;i--) +#if TYPE_OBJECT + if(KEY_EQUALS_NOT_NULL(keys[i]) && EQUALS_VALUE_TYPE(values[i], value)) return true; +#else if(KEY_EQUALS_NOT_NULL(keys[i]) && ((value == null && values[i] == getDefaultReturnValue()) || EQUALS_VALUE_TYPE(values[i], value))) return true; +#endif return false; } @@ -481,7 +483,13 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE insert(-index-1, key, newValue); return newValue; } - return values[index]; + VALUE_TYPE newValue = values[index]; + if(VALUE_EQUALS(newValue, getDefaultReturnValue())) { + newValue = mappingFunction.GET_VALUE(key); + if(VALUE_EQUALS(newValue, getDefaultReturnValue())) return newValue; + values[index] = newValue; + } + return newValue; } @Override @@ -499,7 +507,7 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE @Override public VALUE_TYPE COMPUTE_IF_PRESENT(KEY_TYPE key, UNARY_OPERATOR KEY_VALUE_GENERIC_TYPE mappingFunction) { int index = findIndex(key); - if(index < 0) return getDefaultReturnValue(); + if(index < 0 || VALUE_EQUALS(values[index], getDefaultReturnValue())) return getDefaultReturnValue(); VALUE_TYPE newValue = mappingFunction.APPLY_VALUE(key, values[index]); if(VALUE_EQUALS(newValue, getDefaultReturnValue())) { removeIndex(index); @@ -511,8 +519,12 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE @Override public VALUE_TYPE MERGE(KEY_TYPE key, VALUE_TYPE value, VALUE_UNARY_OPERATOR VALUE_VALUE_GENERIC_TYPE mappingFunction) { + Objects.requireNonNull(mappingFunction); +#if VALUE_OBJECT + Objects.requireNonNull(value); +#endif int index = findIndex(key); - VALUE_TYPE newValue = index < 0 ? value : mappingFunction.APPLY_VALUE(values[index], value); + VALUE_TYPE newValue = index < 0 || VALUE_EQUALS(values[index], getDefaultReturnValue()) ? value : mappingFunction.APPLY_VALUE(values[index], value); if(VALUE_EQUALS(newValue, getDefaultReturnValue())) { if(index >= 0) removeIndex(index); @@ -528,7 +540,7 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE for(MAP.Entry KEY_VALUE_GENERIC_TYPE entry : MAPS.fastIterable(m)) { KEY_TYPE key = entry.ENTRY_KEY(); int index = findIndex(key); - VALUE_TYPE newValue = index < 0 ? entry.ENTRY_VALUE() : mappingFunction.APPLY_VALUE(values[index], entry.ENTRY_VALUE()); + VALUE_TYPE newValue = index < 0 || VALUE_EQUALS(values[index], getDefaultReturnValue()) ? entry.ENTRY_VALUE() : mappingFunction.APPLY_VALUE(values[index], entry.ENTRY_VALUE()); if(VALUE_EQUALS(newValue, getDefaultReturnValue())) { if(index >= 0) removeIndex(index); @@ -744,7 +756,7 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE @Override public String toString() { - return KEY_TO_STRING(keys[index]) + "->" + VALUE_TO_STRING(values[index]); + return KEY_TO_STRING(keys[index]) + "=" + VALUE_TO_STRING(values[index]); } } @@ -928,8 +940,16 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE @Override public boolean contains(Object o) { if(o instanceof Map.Entry) { - if(o instanceof MAP.Entry) return HASH_MAP.this.containsKey(((MAP.Entry KEY_VALUE_GENERIC_TYPE)o).ENTRY_KEY()); - return HASH_MAP.this.containsKey(((Map.Entry)o).getKey()); + if(o instanceof MAP.Entry) { + MAP.Entry KEY_VALUE_GENERIC_TYPE entry = (MAP.Entry KEY_VALUE_GENERIC_TYPE)o; + int index = HASH_MAP.this.findIndex(entry.ENTRY_KEY()); + if(index >= 0) return VALUE_EQUALS(entry.ENTRY_VALUE(), HASH_MAP.this.values[index]); + } + else { + Map.Entry entry = (Map.Entry)o; + int index = HASH_MAP.this.findIndex(entry.getKey()); + if(index >= 0) return Objects.equals(entry.getValue(), VALUE_TO_OBJ(HASH_MAP.this.values[index])); + } } return false; } @@ -958,7 +978,7 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE @Override public boolean remove(Object o) { int oldSize = size; - remove(o); + HASH_MAP.this.remove(o); return size != oldSize; } @@ -971,7 +991,7 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE @Override public boolean remove(KEY_TYPE o) { int oldSize = size; - remove(o); + HASH_MAP.this.remove(o); return size != oldSize; } @@ -1366,9 +1386,9 @@ public class HASH_MAP KEY_VALUE_GENERIC_TYPE extends ABSTRACT_MAP KEY_VALUE_GENE keys[nullIndex] = EMPTY_KEY_VALUE; values[nullIndex] = EMPTY_VALUE; } - else if(pos >= 0) shiftKeys(pos); + else if(lastReturned >= 0) shiftKeys(lastReturned); else { - HASH_MAP.this.remove(wrapped.GET_KEY(-pos - 1)); + HASH_MAP.this.remove(wrapped.GET_KEY(-lastReturned - 1)); return; } size--; diff --git a/src/builder/resources/speiger/assets/collections/templates/maps/interfaces/Map.template b/src/builder/resources/speiger/assets/collections/templates/maps/interfaces/Map.template index 0274a65b..891f0f3f 100644 --- a/src/builder/resources/speiger/assets/collections/templates/maps/interfaces/Map.template +++ b/src/builder/resources/speiger/assets/collections/templates/maps/interfaces/Map.template @@ -1,9 +1,7 @@ package speiger.src.collections.PACKAGE.maps.interfaces; import java.util.Map; -#if VALUE_OBJECT && !TYPE_OBJECT import java.util.Objects; -#endif #if !TYPE_BOOLEAN import java.util.Collection; import java.util.Arrays; @@ -414,30 +412,38 @@ public interface MAP KEY_VALUE_GENERIC_TYPE extends Map mappingFunction) { + Objects.requireNonNull(mappingFunction); REPLACE_VALUES(mappingFunction instanceof UNARY_OPERATOR ? (UNARY_OPERATOR KEY_VALUE_GENERIC_TYPE)mappingFunction : (K, V) -> OBJ_TO_VALUE(mappingFunction.apply(KEY_TO_OBJ(K), VALUE_TO_OBJ(V)))); } @Override @Primitive public default CLASS_VALUE_TYPE compute(CLASS_TYPE key, BiFunction mappingFunction) { + Objects.requireNonNull(mappingFunction); return VALUE_TO_OBJ(COMPUTE(OBJ_TO_KEY(key), mappingFunction instanceof UNARY_OPERATOR ? (UNARY_OPERATOR KEY_VALUE_GENERIC_TYPE)mappingFunction : (K, V) -> OBJ_TO_VALUE(mappingFunction.apply(KEY_TO_OBJ(K), VALUE_TO_OBJ(V))))); } @Override @Primitive public default CLASS_VALUE_TYPE computeIfAbsent(CLASS_TYPE key, Function mappingFunction) { + Objects.requireNonNull(mappingFunction); return VALUE_TO_OBJ(COMPUTE_IF_ABSENT(OBJ_TO_KEY(key), mappingFunction instanceof FUNCTION ? (FUNCTION KEY_VALUE_GENERIC_TYPE)mappingFunction : K -> OBJ_TO_VALUE(mappingFunction.apply(KEY_TO_OBJ(K))))); } @Override @Primitive public default CLASS_VALUE_TYPE computeIfPresent(CLASS_TYPE key, BiFunction mappingFunction) { + Objects.requireNonNull(mappingFunction); return VALUE_TO_OBJ(COMPUTE_IF_PRESENT(OBJ_TO_KEY(key), mappingFunction instanceof UNARY_OPERATOR ? (UNARY_OPERATOR KEY_VALUE_GENERIC_TYPE)mappingFunction : (K, V) -> OBJ_TO_VALUE(mappingFunction.apply(KEY_TO_OBJ(K), VALUE_TO_OBJ(V))))); } @Override @Primitive public default CLASS_VALUE_TYPE merge(CLASS_TYPE key, CLASS_VALUE_TYPE value, BiFunction mappingFunction) { + Objects.requireNonNull(mappingFunction); +#if VALUE_OBJECT + Objects.requireNonNull(value); +#endif return VALUE_TO_OBJ(MERGE(OBJ_TO_KEY(key), OBJ_TO_VALUE(value), mappingFunction instanceof VALUE_UNARY_OPERATOR ? (VALUE_UNARY_OPERATOR VALUE_VALUE_GENERIC_TYPE)mappingFunction : (K, V) -> OBJ_TO_VALUE(mappingFunction.apply(VALUE_TO_OBJ(K), VALUE_TO_OBJ(V))))); } @@ -450,6 +456,7 @@ public interface MAP KEY_VALUE_GENERIC_TYPE extends Map action) { + Objects.requireNonNull(action); forEach(action instanceof BI_CONSUMER ? (BI_CONSUMER KEY_VALUE_GENERIC_TYPE)action : (K, V) -> action.accept(KEY_TO_OBJ(K), VALUE_TO_OBJ(V))); } diff --git a/src/builder/resources/speiger/assets/collections/templates/sets/AbstractSet.template b/src/builder/resources/speiger/assets/collections/templates/sets/AbstractSet.template index 74db7e29..14d5f84c 100644 --- a/src/builder/resources/speiger/assets/collections/templates/sets/AbstractSet.template +++ b/src/builder/resources/speiger/assets/collections/templates/sets/AbstractSet.template @@ -1,7 +1,8 @@ package speiger.src.collections.PACKAGE.sets; -import java.util.Iterator; +#if TYPE_OBJECT import java.util.Objects; +#endif import java.util.Set; import speiger.src.collections.PACKAGE.collections.ABSTRACT_COLLECTION; @@ -20,10 +21,10 @@ public abstract class ABSTRACT_SET KEY_GENERIC_TYPE extends ABSTRACT_COLLECTION @Override public int hashCode() { - int hashCode = 1; + int hashCode = 0; ITERATOR KEY_GENERIC_TYPE i = iterator(); while(i.hasNext()) - hashCode = 31 * hashCode + KEY_TO_HASH(i.NEXT()); + hashCode += KEY_TO_HASH(i.NEXT()); return hashCode; } @@ -35,24 +36,10 @@ public abstract class ABSTRACT_SET KEY_GENERIC_TYPE extends ABSTRACT_COLLECTION return false; Set l = (Set)o; if(l.size() != size()) return false; -#if !TYPE_OBJECT - if(l instanceof SET) - { - ITERATOR e1 = iterator(); - ITERATOR e2 = ((SET)l).iterator(); - while (e1.hasNext() && e2.hasNext()) { - if(!(KEY_EQUALS(e1.NEXT(), e2.NEXT()))) - return false; - } - return !(e1.hasNext() || e2.hasNext()); + try { + return containsAll(l); + } catch (ClassCastException | NullPointerException unused) { + return false; } -#endif - Iterator e1 = iterator(); - Iterator e2 = l.iterator(); - while (e1.hasNext() && e2.hasNext()) { - if(!Objects.equals(e1.next(), e2.next())) - return false; - } - return !(e1.hasNext() || e2.hasNext()); } } diff --git a/src/test/java/speiger/src/collections/objects/map/MapTest.java b/src/test/java/speiger/src/collections/objects/map/MapTest.java new file mode 100644 index 00000000..3f1d8127 --- /dev/null +++ b/src/test/java/speiger/src/collections/objects/map/MapTest.java @@ -0,0 +1,40 @@ +package speiger.src.collections.objects.map; + +import java.util.Map; +import java.util.function.Supplier; + +import com.google.common.collect.testing.MapTestSuiteBuilder; +import com.google.common.collect.testing.TestStringMapGenerator; +import com.google.common.collect.testing.features.CollectionFeature; +import com.google.common.collect.testing.features.CollectionSize; +import com.google.common.collect.testing.features.MapFeature; + +import junit.framework.Test; +import junit.framework.TestCase; +import speiger.src.collections.objects.maps.impl.hash.Object2ObjectOpenHashMap; + +@SuppressWarnings("javadoc") +public final class MapTest extends TestCase +{ + public static Test suite() + { + return suite("Object2ObjectOpenHashMap", Object2ObjectOpenHashMap::new); + } + + public static Test suite(String name, Supplier> factory) + { + return MapTestSuiteBuilder.using(new TestStringMapGenerator() + { + @Override + protected Map create(Map.Entry[] entries) + { + Map map = factory.get(); + for(Map.Entry entry : entries) + { + map.put(entry.getKey(), entry.getValue()); + } + return map; + } + }).named(name).withFeatures(CollectionSize.ANY, MapFeature.GENERAL_PURPOSE, MapFeature.ALLOWS_NULL_KEYS, MapFeature.ALLOWS_NULL_VALUES, MapFeature.ALLOWS_ANY_NULL_QUERIES, CollectionFeature.SUPPORTS_ITERATOR_REMOVE).createTestSuite(); + } +}