Re: Suggestion for a more sensible implementation of EMPTY_MAP
> On 27 Jun 2019, at 22:38, Stuart Marks wrote:
>
>
>
> On 6/19/19 12:51 AM, Abraham Marín Pérez wrote:
>> private void decorate(Map data) {
>> //...
>> data.computeIfPresent("field", (k, v) -> highlightDifferences(v,
>> otherValue));
>> //...
>> }
>> At one point an emptyMap() was passed to this method, causing an UOE. [...]
>
> On the face of it, the decorate() method has the possibility to modify the
> map that it's passed. Thus, it shouldn't be at all surprising that passing an
> unmodifiable map to it results in UOE.
decorate() does other things other than calling computeIfPresent. Given our
business logic, we don’t believe throwing UOE is a helpful behaviour in this
particular case.
>
> As a special case, it doesn't *actually* modify the map if "field" is absent,
> but you have to do some analysis to figure this out.
>
> Now you want the JDK to add another special case to Collections.emptyMap(),
> and possibly others, in order to make this special case work. I don't think
> we want to do that.
Fair enough. I made the suggestion thinking that this is something that would
benefit the overall community. But maybe that’s not the case. If the overall
community benefits better from the current implementation, then I agree it
should stay as it is.
Thanks for taking the time to debate and consider this change.
Cheers,
Abraham
>
> s'marks
Re: Suggestion for a more sensible implementation of EMPTY_MAP
On 6/19/19 12:51 AM, Abraham Marín Pérez wrote:
private void decorate(Map data) {
//...
data.computeIfPresent("field", (k, v) -> highlightDifferences(v,
otherValue));
//...
}
At one point an emptyMap() was passed to this method, causing an UOE. [...]
On the face of it, the decorate() method has the possibility to modify the map
that it's passed. Thus, it shouldn't be at all surprising that passing an
unmodifiable map to it results in UOE.
As a special case, it doesn't *actually* modify the map if "field" is absent,
but you have to do some analysis to figure this out.
Now you want the JDK to add another special case to Collections.emptyMap(), and
possibly others, in order to make this special case work. I don't think we want
to do that.
s'marks
Re: Suggestion for a more sensible implementation of EMPTY_MAP
I just realised I sent my last reply to Anthony only... apologies. Please
see below.
Cheers,
Abraham
El mié., 19 jun. 2019 a las 17:43, Abraham Marin-Perez (<
abraham.marin.pe...@gmail.com>) escribió:
> Hi Anthony,
>
> There is code before and after compareIfPresent, but yeah, we could have
> reordered that code and include an isEmpty check to avoid the problem.
> However, let’s take a step back and have a look at what this entails.
>
> Having documentation is great, needing documentation not so much. Whenever
> possible, methods should have a name that describes in plain English what
> they do so as to minimise the need to go and check the documentation. This
> makes code intuitive. Let’s analyse the plain English for computeIfPresent.
> Compute if present. It doesn’t say what happens if not present, but one
> would argue that a plain English interpretation would be “it not present,
> nothing happens”.
>
> We’ve already seen that there are some powerful reasons for which this is
> not quite the case (as Roger laid out in detail). Reasons that lead us to
> the unfortunate situation that the plain English interpretation is not
> quite correct, but calling it
> “computeIfPresentAndIfNotNothingHappensUnlessTheMapIsImmutable” would be
> absurd, so we live with that. The side effect is we have a method whose
> behaviour causes confusion to developers and forces them to check the
> documentation.
>
> Once the developer has checked the documentation, then we need to adapt
> the code and add some guards that, on face value, shouldn’t be needed. Why
> should you need to check whether a map is empty before attempting
> computeIfPresent? You shouldn’t, the fact that you do is just a side effect
> of the an implementation decision.
>
> So my point is: we could definitely do what you suggested, but I am not
> entirely sure that such measures should be needed.
>
> Cheers,
> Abraham
>
> Sent from my iPhone
>
> On 19 Jun 2019, at 11:58, Anthony Vanelverdinghe
> wrote:
>
> Hi Abraham
>
>
>
> Wouldn’t it have been possible to use Map::isEmpty? Ideally just returning
> at the very beginning of your method if `data` is an empty Map. That way
> you’d still be able to use computeIfPresent, and use emptyMap() as well.
>
>
>
> Kind regards,
>
> Anthony
>
>
> --
> *From:* core-libs-dev on behalf
> of Abraham Marín Pérez
> *Sent:* Wednesday, June 19, 2019 9:51:46 AM
> *To:* Roger Riggs
> *Cc:* core-libs-dev@openjdk.java.net
> *Subject:* Re: Suggestion for a more sensible implementation of EMPTY_MAP
>
> Hi Stuart, Roger,
>
> First of all, thank you for such a detailed response, this really shows
> the big picture. I guess no implementation will be perfect, there will
> always be some wrinkles that we need to accept, and the most suitable
> implementation will be the one with the fewest or least impacting wrinkles.
> Given this, maybe showing the particular wrinkle that I faced can shed
> light on the overall development experience that the current implementation
> provides.
>
> For multiple reasons that I cannot explain at this point, I have a method
> that looks like the following:
>
> private void decorate(Map data) {
> //...
> data.computeIfPresent("field", (k, v) -> highlightDifferences(v,
> otherValue));
> //...
> }
>
> At one point an emptyMap() was passed to this method, causing an UOE. This
> left me with two choices:
>
> 1. Change code to:
>
> if(data.hasKey(“field"))
> data.compute("field", (k, v) -> highlightDifferences(v, otherValue));
>
> 2. Avoid usage of emptyMap() and use new HashMap<>() instead
>
>
> The first option defeats the purpose of having a computeIfPresent method,
> and causes confusion among team members (people keep asking why
> computeIfPresent isn't used). The second option is pretty much what Roger
> mentioned (and what we ended up doing).
>
> On the other hand, there are some points that you mentioned that I believe
> merit some extra debate, please find some comments below inline.
>
> In general, I tend to agree that a stricter implementation is better since
> it usually prevents bugs. However, at least in this particular case, I
> believe a stricter implementation also has the side effect of worsening the
> development experience and adding confusion (see below comments). Of
> course, as I mentioned before, I understand that no implementation will be
> perfect, and maybe the current option is the lesser evil, but I wanted to
> throw in an alternative for consideration.
>
> Many thanks,
> Abraham
>
>
> El vie., 14 jun. 2019 a las 14:18, Roger Riggs ( mailto:roger.ri..
RE: Suggestion for a more sensible implementation of EMPTY_MAP
Hi Abraham
Wouldn’t it have been possible to use Map::isEmpty? Ideally just returning at
the very beginning of your method if `data` is an empty Map. That way you’d
still be able to use computeIfPresent, and use emptyMap() as well.
Kind regards,
Anthony
From: core-libs-dev on behalf of
Abraham Marín Pérez
Sent: Wednesday, June 19, 2019 9:51:46 AM
To: Roger Riggs
Cc: core-libs-dev@openjdk.java.net
Subject: Re: Suggestion for a more sensible implementation of EMPTY_MAP
Hi Stuart, Roger,
First of all, thank you for such a detailed response, this really shows the big
picture. I guess no implementation will be perfect, there will always be some
wrinkles that we need to accept, and the most suitable implementation will be
the one with the fewest or least impacting wrinkles. Given this, maybe showing
the particular wrinkle that I faced can shed light on the overall development
experience that the current implementation provides.
For multiple reasons that I cannot explain at this point, I have a method that
looks like the following:
private void decorate(Map data) {
//...
data.computeIfPresent("field", (k, v) -> highlightDifferences(v,
otherValue));
//...
}
At one point an emptyMap() was passed to this method, causing an UOE. This left
me with two choices:
1. Change code to:
if(data.hasKey(“field"))
data.compute("field", (k, v) -> highlightDifferences(v, otherValue));
2. Avoid usage of emptyMap() and use new HashMap<>() instead
The first option defeats the purpose of having a computeIfPresent method, and
causes confusion among team members (people keep asking why computeIfPresent
isn't used). The second option is pretty much what Roger mentioned (and what we
ended up doing).
On the other hand, there are some points that you mentioned that I believe
merit some extra debate, please find some comments below inline.
In general, I tend to agree that a stricter implementation is better since it
usually prevents bugs. However, at least in this particular case, I believe a
stricter implementation also has the side effect of worsening the development
experience and adding confusion (see below comments). Of course, as I mentioned
before, I understand that no implementation will be perfect, and maybe the
current option is the lesser evil, but I wanted to throw in an alternative for
consideration.
Many thanks,
Abraham
El vie., 14 jun. 2019 a las 14:18, Roger Riggs (mailto:roger.ri...@oracle.com>>) escribió:
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 map1 = Collections.emptyMap();
> map1.putAll(otherMap);
>
> The answer is that it depends on th
Re: Suggestion for a more sensible implementation of EMPTY_MAP
Hi Stuart, Roger,
First of all, thank you for such a detailed response, this really shows the big
picture. I guess no implementation will be perfect, there will always be some
wrinkles that we need to accept, and the most suitable implementation will be
the one with the fewest or least impacting wrinkles. Given this, maybe showing
the particular wrinkle that I faced can shed light on the overall development
experience that the current implementation provides.
For multiple reasons that I cannot explain at this point, I have a method that
looks like the following:
private void decorate(Map data) {
//...
data.computeIfPresent("field", (k, v) -> highlightDifferences(v,
otherValue));
//...
}
At one point an emptyMap() was passed to this method, causing an UOE. This left
me with two choices:
1. Change code to:
if(data.hasKey(“field"))
data.compute("field", (k, v) -> highlightDifferences(v, otherValue));
2. Avoid usage of emptyMap() and use new HashMap<>() instead
The first option defeats the purpose of having a computeIfPresent method, and
causes confusion among team members (people keep asking why computeIfPresent
isn't used). The second option is pretty much what Roger mentioned (and what we
ended up doing).
On the other hand, there are some points that you mentioned that I believe
merit some extra debate, please find some comments below inline.
In general, I tend to agree that a stricter implementation is better since it
usually prevents bugs. However, at least in this particular case, I believe a
stricter implementation also has the side effect of worsening the development
experience and adding confusion (see below comments). Of course, as I mentioned
before, I understand that no implementation will be perfect, and maybe the
current option is the lesser evil, but I wanted to throw in an alternative for
consideration.
Many thanks,
Abraham
El vie., 14 jun. 2019 a las 14:18, Roger Riggs (mailto:roger.ri...@oracle.com>>) escribió:
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 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 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
Re: Suggestion for a more sensible implementation of EMPTY_MAP
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 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 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 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 map1 = Collections.emptyMap();
Map map2 = Map.of();
Map 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 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
Re: Suggestion for a more sensible implementation of EMPTY_MAP
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 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 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 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 map1 = Collections.emptyMap();
Map map2 = Map.of();
Map 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 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
Re: Suggestion for a more sensible implementation of EMPTY_MAP
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 > 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
Re: Suggestion for a more sensible implementation of EMPTY_MAP
> 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
