Bug 576333 - Simplify ConcurrentNeutralValueMap
Fixed and added test for toString, hashCode, equals just in case it
would ever be used.
Change-Id: I4a55101530c3ce813dd80ebdba05d4b2dd867c6f
Signed-off-by: Joerg Kubitz <jkubitz-eclipse@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.runtime/+/186070
Reviewed-by: Sebastian Zarnekow <sebastian.zarnekow@gmail.com>
Tested-by: Platform Bot <platform-bot@eclipse.org>
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java
index f588fac..01e898e 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java
@@ -27,6 +27,7 @@
final ConcurrentHashMap<K, V> delegate = new ConcurrentHashMap<>();
/** a value that is used for null elements **/
final private V neutralValue;
+ final private static NullValue NULL = new NullValue();
/**
* @param neutralValue a Element that does not equal any other Value
@@ -36,6 +37,11 @@
this.neutralValue = neutralValue;
}
+ @SuppressWarnings("unchecked")
+ public ConcurrentNeutralValueMap() {
+ this((V) NULL);
+ }
+
private V wrapValue(V value) {
return value == null ? neutralValue : value;
}
@@ -49,6 +55,7 @@
}
private static final class NullValue {
+ /** for {@link ConcurrentNeutralValueMap#toString()} **/
@Override
public String toString() {
return "null"; //$NON-NLS-1$
@@ -56,34 +63,42 @@
@Override
public boolean equals(Object obj) {
- return false;
+ // currently not called since ConcurrentHashMap checks for value1==value2
+ // but just in case the Implementation changes:
+ // in means of ConcurrentNeutralValueMap#equals() every NullValue equals
+ return obj instanceof NullValue;
+ // like java.util.Objects.equals(null, null) would return true,
}
+ /** for {@link ConcurrentNeutralValueMap#hashCode()} **/
@Override
public int hashCode() {
+ // in means of ConcurrentNeutralValueMap#hashCode() we need a hashCode
+ // even tough null.hashCode() would normally throw NullPointerException.
return 0x55555555;
+ // like java.util.Objects.hashCode(null) would return an int,
}
}
- /** returns a neutral Element that does not equals any other **/
- public static Object neutralObject() {
- return new NullValue();
- }
-
/*
* Method overloads from java.lang.Object:
*/
+ /** return uses hashCode() of the neutralValue **/
@Override
public int hashCode() {
return delegate.hashCode();
}
+ /** return does NOT use equals() of the neutralValue **/
@Override
public boolean equals(Object obj) {
- return delegate.equals(obj);
+ if (!(obj instanceof ConcurrentNeutralValueMap))
+ return false;
+ return delegate.equals(((ConcurrentNeutralValueMap<?, ?>) obj).delegate);
}
+ /** return uses toString() of the neutralValue **/
@Override
public String toString() {
return delegate.toString();
@@ -186,9 +201,12 @@
boolean isPresent();
/**
- * @return the result of map.get(key)
+ * Callers are supposed to check #isPresent first to determine whether a
+ * returned null was a null value or the key was not present.
+ *
+ * @return the unwrapped result of map.get(key). May return null.
*/
- V unwraped();
+ V unwrapped();
}
private final class Wrapped implements Value<V> {
@@ -204,7 +222,7 @@
}
@Override
- public V unwraped() {
+ public V unwrapped() {
return unwrapValue(wrappedValue);
}
}
@@ -212,7 +230,7 @@
/**
* @return a value that is wrapped into a Value Object.
* @see Value#isPresent()
- * @see Value#unwraped()
+ * @see Value#unwrapped()
*/
public Value<V> getValue(K key) {
return new Wrapped(delegate.get(key));
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
index e8b7311..c777db8 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
@@ -95,7 +95,7 @@
private Map<String, ValueComputation> localValueComputations = new ConcurrentHashMap<>();
final protected ConcurrentNeutralValueMap<String, Object> localValues = // null values allowed
- new ConcurrentNeutralValueMap<>(ConcurrentNeutralValueMap.neutralObject());
+ new ConcurrentNeutralValueMap<>();
private Set<String> modifiable;
@@ -249,7 +249,7 @@
// 1. try for local value
Value<Object> value = localValues.getValue(name);
if (value.isPresent()) {
- result = value.unwraped();
+ result = value.unwrapped();
if (result == null)
return null;
} else
@@ -314,7 +314,7 @@
Value<Object> value = localValues.getValue(name);
if (!value.isPresent())
return false;
- return (value.unwraped() == newValue);
+ return (value.unwrapped() == newValue);
}
private boolean isSetLocally(String name) {
@@ -365,7 +365,7 @@
}
Value<Object> old = localValues.putAndGetOld(name, value);
boolean containsKey = old.isPresent();
- Object oldValue = old.unwraped();
+ Object oldValue = old.unwrapped();
if (!containsKey || oldValue != value) {
Set<Scheduled> scheduled = new LinkedHashSet<>();
invalidate(name, ContextChangeEvent.ADDED, oldValue, value, scheduled);
@@ -400,7 +400,7 @@
String tmp = "Variable " + name + " is not modifiable in the context " + this; //$NON-NLS-1$ //$NON-NLS-2$
throw new IllegalArgumentException(tmp);
}
- Object oldValue = localValues.putAndGetOld(name, value).unwraped();
+ Object oldValue = localValues.putAndGetOld(name, value).unwrapped();
if (oldValue != value)
invalidate(name, ContextChangeEvent.ADDED, oldValue, value, scheduled);
return true;
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java
index dccb2c3..0217012 100644
--- a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java
@@ -28,14 +28,6 @@
public class NeutralValueTest {
@Test
- public void testNeutralObject() {
- assertFalse(new Object().equals(ConcurrentNeutralValueMap.neutralObject()));
- assertFalse(ConcurrentNeutralValueMap.neutralObject().equals(new Object()));
- assertNotEquals(ConcurrentNeutralValueMap.neutralObject(), ConcurrentNeutralValueMap.neutralObject());
- assertEquals("" + null, ConcurrentNeutralValueMap.neutralObject().toString());
- }
-
- @Test
public void testConcurrentNeutralValueMap() {
ConcurrentNeutralValueMap<String, Double> map = new ConcurrentNeutralValueMap<>(Double.NaN);
map.put("nix", null); // modify
@@ -78,29 +70,29 @@
{
Value<Double> v = map.getValue("nix");
assertTrue(v.isPresent());
- assertEquals(null, v.unwraped());
+ assertEquals(null, v.unwrapped());
}
{
Value<Double> v = map.getValue("1");
assertFalse(v.isPresent());
- assertEquals(null, v.unwraped());
+ assertEquals(null, v.unwrapped());
}
{
Value<Double> v = map.getValue("2");
assertTrue(v.isPresent());
- assertEquals(Double.valueOf(2.0), v.unwraped());
+ assertEquals(Double.valueOf(2.0), v.unwrapped());
}
{
Value<Double> v = map.putAndGetOld("5", 5555.0); // modify
assertFalse(v.isPresent());
- assertEquals(null, v.unwraped());
+ assertEquals(null, v.unwrapped());
assertEquals(Double.valueOf(5555.0), map.get("5"));
}
{
Value<Double> v = map.putAndGetOld("5", 5.0); // modify
assertTrue(v.isPresent());
- assertEquals(Double.valueOf(5555.0), v.unwraped());
+ assertEquals(Double.valueOf(5555.0), v.unwrapped());
assertEquals(Double.valueOf(5.0), map.get("5"));
}
map.putIfAbsent("5", 5555.0); // modify
@@ -111,27 +103,27 @@
{
Value<Double> v = map.putAndGetOld("five", null); // modify
assertFalse(v.isPresent());
- assertEquals(null, v.unwraped());
+ assertEquals(null, v.unwrapped());
assertEquals(null, map.get("five"));
}
{
Value<Double> v = map.putAndGetOld("five", 5.0); // modify
assertTrue(v.isPresent());
- assertEquals(null, v.unwraped());
+ assertEquals(null, v.unwrapped());
assertEquals(Double.valueOf(5.0), map.get("five"));
}
{
map.putIfAbsent("five", null); // modify
Value<Double> v = map.getValue("five");
assertTrue(v.isPresent());
- assertEquals(Double.valueOf(5.0), v.unwraped());
+ assertEquals(Double.valueOf(5.0), v.unwrapped());
}
map.remove("five"); // modify
{
map.putIfAbsent("five", null); // modify
Value<Double> v = map.getValue("five");
assertTrue(v.isPresent());
- assertEquals(null, v.unwraped());
+ assertEquals(null, v.unwrapped());
}
map.remove("five"); // modify
assertFalse(map.containsKey("five"));
@@ -141,4 +133,73 @@
assertTrue(map.isEmpty());
}
+ @Test
+ public void testToString() {
+ ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>();
+ map1.put("0", 0f);
+ map1.put("NULL", null);
+ map1.put("nothing", null);
+ map1.put("1", 1f);
+ map1.put("~2", 2.1f);
+ assertTrue(map1.toString().contains("0=0.0"));
+ assertTrue(map1.toString().contains("1=1.0"));
+ assertTrue(map1.toString().contains("~2=2.1"));
+ assertTrue(map1.toString().contains("NULL=null"));
+ assertTrue(map1.toString().contains("nothing=null"));
+ }
+
+ @Test
+ public void testCustomToString() {
+ ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(Float.NaN);
+ map1.put("0", 0f);
+ map1.put("NULL", null);
+ assertTrue(map1.toString().contains("0=0.0"));
+ assertTrue(map1.toString().contains("NULL=NaN"));
+ }
+
+ @Test
+ public void testEquals() {
+ {
+ ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>();
+ ConcurrentNeutralValueMap<String, Float> map2 = new ConcurrentNeutralValueMap<>();
+ map1.put("0", 0f);
+ map1.put("NULL", null);
+ map2.put("NULL", null);
+ map2.put("0", 0f);
+ assertEquals(map1.hashCode(), map2.hashCode());
+ assertEquals(map1, map2);
+ }
+ {
+ ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>();
+ ConcurrentNeutralValueMap<String, Float> map2 = new ConcurrentNeutralValueMap<>();
+ map1.put("0", 0f);
+ map1.put("1", 1f);
+ map1.put("NULL", null);
+ map2.put("NULL", null);
+ map2.put("0", 0f);
+ assertNotEquals(map1.hashCode(), map2.hashCode());
+ assertNotEquals(map1, map2);
+ }
+ {
+ ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(Float.NaN);
+ ConcurrentNeutralValueMap<String, Float> map2 = new ConcurrentNeutralValueMap<>(Float.NaN);
+ map1.put("0", 0f);
+ map1.put("NULL", null);
+ map2.put("NULL", null);
+ map2.put("0", 0f);
+ assertEquals(map1.hashCode(), map2.hashCode());
+ assertEquals(map1, map2);
+ }
+ {
+ ConcurrentNeutralValueMap<String, Float> map1 = new ConcurrentNeutralValueMap<>(Float.NaN);
+ ConcurrentNeutralValueMap<String, Float> map2 = new ConcurrentNeutralValueMap<>(Float.NaN);
+ map1.put("0", 0f);
+ map1.put("1", 1f);
+ map1.put("NULL", null);
+ map2.put("NULL", null);
+ map2.put("0", 0f);
+ assertNotEquals(map1.hashCode(), map2.hashCode());
+ assertNotEquals(map1, map2);
+ }
+ }
}
