Hi Stuart,
Not withstanding all the details. It would be useful (and possibly
expected) that an EMPTY_MAP
could be substituted for a Map with no entries. As it stands, the
caller needs know whether any optional
possibly modifying operations will be used and decide whether to create
an empty map or an EMPTY_MAP.
That makes using an EMPTY_MAP a risk and less useful and a cautious
programmer will avoid it.
$.02, Roger
On 6/13/19 8:42 PM, Stuart Marks wrote:
On 6/10/19 7:34 AM, Abraham Marín Pérez wrote:
I agree that it makes sense for EMPTY_MAP to have the same logic as
Map.of() or unmodifiable(new HashMap()), which means that my
suggestion cannot be applied to just EMPTY_MAP... but maybe that’s
ok: maybe we can change all of them? If we keep the default
implementation for Map.computeIfPresent in EMPTY_MAP, Map.of(),
unmodifiableMap, etc., instead of overriding it to throw an
exception, then:
- For cases where the key isn’t present (regardless of empty or
non-empt map), the call will be a safe no-op.
- For cases where the key is present, the call will still throw UOE
via the implementation of put()
I believe this would still respect the spec for Map.computeIfPresent
while providing a more forgiving implementation (in the sense that it
will avoid throwing an exception when possible).
Hi Abraham,
The specification does not mandate one behavior or another. Either
behavior is permitted, and in fact both behaviors are present in the JDK.
The second and more significant point is raised by your last statement
suggesting a "more forgiving implementation." The question is, do we
actually want a more forgiving implementation?
The collections specs define certain methods as "optional", but it's
not clear whether this means that calling such a method should always
throw an exception ("strict" behavior) or whether the method should
throw an exception only when it attempts to do something that's
disallowed ("lenient" behavior).
Take for example the putAll() method. What happens if we do this?
Map<String, String> map1 = Collections.emptyMap();
map1.putAll(otherMap);
The answer is that it depends on the state of otherMap. If otherMap is
empty, then the operation succeeds (and does nothing). However, if
otherMap is non-empty, then the operation will throw
UnsupportedOperationException. That's an example of lenient behavior.
What's notable about this is that you can't predict the outcome unless
you know the state of otherMap.
Now, how about this example?
Map<String, String> map2 = Map.of();
map2.putAll(otherMap);
This always throws UnsupportedOperationException, regardless of the
state of otherMap. I'd call this strict behavior.
More recent implementations, including the Java 8 default methods, and
the Java 9 unmodifiable collections (Map.of et al), have all preferred
strict behavior. This is because less information about the state of
the system is necessary in order to reason about the code, which leads
to fewer bugs, or at least earlier detection of bugs. It's unfortunate
that emptyMap().putAll() has this lenient behavior, but we're stuck
with it for compatibility reasons.
Now there is a slight wrinkle with computeIfPresent(). If we have
Map<String, String> map = Collections.emptyMap();
map.computeIfPresent(key, remappingFunction);
then as you point out, this can never modify the map, since the key is
never present. Thus, there isn't any behavior that's dependent upon
additional state.
But, as Michael observed, there is now an inconsistency with Map.of()
and Collections.unmodifiableMap(). So, continuing with your
suggestion, we might change those implementations to allow
computeIfPresent() as well. Thus,
Map<String, String> map1 = Collections.emptyMap();
Map<String, String> map2 = Map.of();
Map<String, String> map3 = Collections.unmodifiableMap(new
HashMap<>());
and then
{map1,map2,map3}.computeIfPresent(key, remappingFunction);
all are no-ops. Well, maybe. What if we had retained a reference to
the HashMap wrapped by the unmodifiableMap, and then we added an
element? Now it contains a mapping; what should happen then? Should
computeIfPresent() throw an exception because it *might* attempt to
modify the map? Or should it throw an exception *only* if it attempts
to modify the map? State-dependent behavior has slipped back in.
We also need to consider the impact on the other compute* methods.
Consider:
Map<String, String> map = Collections.emptyMap();
map.compute(key, (k, v) -> null);
This can never change the map, but it currently throws UOE. Should it
be changed to a no-op as well?
**
What we have now is strict rule: calling mutator methods on
unmodifiable collections throws an exception. It's simple to describe
and reason about, and it's (mostly) consistent across various
collections. However, it prohibits some operations that sometimes
people want to do.
If we were to make the system more lenient, or more forgiving, then
weird inconsistencies and conditional behaviors pop up in other
places. This makes the system more complicated and harder to reason
about, and that leads to more bugs.
s'marks
