Suggestion for a more sensible implementation of EMPTY_MAP

[Abraham Marín Pérez]

Hi Michael,

Many thanks for your reply, I can definitely see your point.

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).

Thoughts?
Abraham


> On 10 Jun 2019, at 14:27, Michael Rasmussen <Michael.Rasmussen at roguewave.com> wrote:
> 
>> If I understand correctly, this class represents an immutable empty map. As
>> a result, operations like put or remove have been implemented to throw
>> UnsupportedOperationException; this makes sense to me. However, this is
>> also the implementation for computeIfPresent, which I believe may be too
>> disruptive: the definition of this method says “If the value for the
>> specified key is present and non-null, attempts to compute a new mapping
>> given the key and its current mapped value.“; for an empty map, this could
>> be a safe no-op, instead of an exception.
> 
> The spec for Map.computeIfPresent states that it should throw UnsupportedOperationException if Map.put is not supported, which is the case for this map.
> The exception is listed as "(optional)" though, for which the docs specify "may throw an exception or it may succeed, at the option of the implementation." so it's not entirely clear to me if it would be OK for computeIfPresent to not throw.
> 
> That being said -- for me it makes more sense that Collections.emptyMap() follows the same logic as Map.of(), or Collections.unmodifiableMap​(new HashMap<>()) -- and the latter two both throw from computeIfPresent.
> 
> /Michael