Redundant assignment in Map's putIfAbsent implementation
up vote
2
down vote
favorite
Looking at the implementation of the default method putIfAbsent
in the interface Map
,
default V putIfAbsent(K key, V value)
V v = get(key);
if (v == null)
v = put(key, value);
return v;
I wonder why the assignment
v = put(key, value);
was done there instead of simply discarding the returned value? This assignment seems unnecessary because v
is already null
, and that is what the put
method, according to its contract, always returns in this case.
java
add a comment |
up vote
2
down vote
favorite
Looking at the implementation of the default method putIfAbsent
in the interface Map
,
default V putIfAbsent(K key, V value)
V v = get(key);
if (v == null)
v = put(key, value);
return v;
I wonder why the assignment
v = put(key, value);
was done there instead of simply discarding the returned value? This assignment seems unnecessary because v
is already null
, and that is what the put
method, according to its contract, always returns in this case.
java
1
My guess is that it accounts for possibility of a race condition. This method is not an atomic action. The key value association may have been updated in between theget
andput
calls
– flakes
Nov 10 at 3:04
I thought about concurrency over-cautions, but the phrase "The default implementation makes no guarantees about synchronization or atomicity properties of this method" in the documentation ofputIfAbsent
suggests otherwise. Maybe it is an attempt to be on the safe side, as mentioned in answers.
– John McClane
Nov 10 at 3:22
add a comment |
up vote
2
down vote
favorite
up vote
2
down vote
favorite
Looking at the implementation of the default method putIfAbsent
in the interface Map
,
default V putIfAbsent(K key, V value)
V v = get(key);
if (v == null)
v = put(key, value);
return v;
I wonder why the assignment
v = put(key, value);
was done there instead of simply discarding the returned value? This assignment seems unnecessary because v
is already null
, and that is what the put
method, according to its contract, always returns in this case.
java
Looking at the implementation of the default method putIfAbsent
in the interface Map
,
default V putIfAbsent(K key, V value)
V v = get(key);
if (v == null)
v = put(key, value);
return v;
I wonder why the assignment
v = put(key, value);
was done there instead of simply discarding the returned value? This assignment seems unnecessary because v
is already null
, and that is what the put
method, according to its contract, always returns in this case.
java
java
asked Nov 10 at 2:51
John McClane
33312
33312
1
My guess is that it accounts for possibility of a race condition. This method is not an atomic action. The key value association may have been updated in between theget
andput
calls
– flakes
Nov 10 at 3:04
I thought about concurrency over-cautions, but the phrase "The default implementation makes no guarantees about synchronization or atomicity properties of this method" in the documentation ofputIfAbsent
suggests otherwise. Maybe it is an attempt to be on the safe side, as mentioned in answers.
– John McClane
Nov 10 at 3:22
add a comment |
1
My guess is that it accounts for possibility of a race condition. This method is not an atomic action. The key value association may have been updated in between theget
andput
calls
– flakes
Nov 10 at 3:04
I thought about concurrency over-cautions, but the phrase "The default implementation makes no guarantees about synchronization or atomicity properties of this method" in the documentation ofputIfAbsent
suggests otherwise. Maybe it is an attempt to be on the safe side, as mentioned in answers.
– John McClane
Nov 10 at 3:22
1
1
My guess is that it accounts for possibility of a race condition. This method is not an atomic action. The key value association may have been updated in between the
get
and put
calls– flakes
Nov 10 at 3:04
My guess is that it accounts for possibility of a race condition. This method is not an atomic action. The key value association may have been updated in between the
get
and put
calls– flakes
Nov 10 at 3:04
I thought about concurrency over-cautions, but the phrase "The default implementation makes no guarantees about synchronization or atomicity properties of this method" in the documentation of
putIfAbsent
suggests otherwise. Maybe it is an attempt to be on the safe side, as mentioned in answers.– John McClane
Nov 10 at 3:22
I thought about concurrency over-cautions, but the phrase "The default implementation makes no guarantees about synchronization or atomicity properties of this method" in the documentation of
putIfAbsent
suggests otherwise. Maybe it is an attempt to be on the safe side, as mentioned in answers.– John McClane
Nov 10 at 3:22
add a comment |
2 Answers
2
active
oldest
votes
up vote
5
down vote
accepted
The implementation is like that because the javadoc states it should be:
Implementation Requirements:
The default implementation is equivalent to, for this map:
V v = map.get(key);
if (v == null)
v = map.put(key, value);
return v;
But why is it specified like that? Probably to get reasonably well-defined behavior in the presence of race conditions that cannot be addressed1.
First, here's what the javadoc says about it.
The default implementation makes no guarantees about synchronization or atomicity properties of this method. Any implementation providing atomicity guarantees must override this method and document its concurrency properties.
So how will the above implementation behave?
If there is no race with another thread mutating that map entry, it will either return
null
or the previous value (without updating the map).If there is a race and another thread updates the entry immediately after the
get
call, then themap.put
call will NOT returnnull
. Instead, it will actually return the value that the other thread inserted. And that value will be returned to the caller.
Note that this is not exactly in conformance with the primary description for the putIfAbsent
method. But this is excused by the "no guarantees" statement. And furthermore, it kind of makes sense.
Is this useful? Well ... probably yes, if the actual Map
method is thread safe. Because, if the calling code wanted to look, this would allow it to detect that a race had occurred and (if it made sense in the context of the overall application design) try to do something about it.
1 - The race conditions cannot be addressed in a default method which has no knowledge of the behavior of implementation classes. They could be addressed in the implementation classes themselves ... by overriding the method to have atomic properties, for example. And the method spec in the Map
javadocs clearly flag this possibility.
But the bottom line is that that supposedly redundant assignment is there because the spec clearly says that it should be there.
Wouldn't it be better design decision to change the linev = put(key, value);
toif (put(key, value) != null) throw new ConcurrentModificationException();
?
– John McClane
Nov 10 at 3:49
Nope. A concurrent collection doesn't throw that exception. And besides, as I explained, the specified behavior is predictable and useful. But it really doesn't alter anything debating if it could have been designed better.
– Stephen C
Nov 10 at 4:02
add a comment |
up vote
3
down vote
It must always return the old value.
If the map is accessed by multiple threads, the value returned by put
may differ from the value returned by get
, so assigning to v
ensures better behavior.
Of course, the code isn't great for handling multi-threaded access, so any valid thread-safe implementation overrides the method to do it better, but this is the best that a generic default implementation can achieve.
IMO this is a poor implementation decision. If the value fromput
is different fromget
, then the method still updates the key-value pair, and further, the return value is non-null, which normally indicates that theput
did not actually happen.
– flakes
Nov 10 at 3:10
1
It is not an implementation decision. It is a specification decision. See my answer.
– Stephen C
Nov 10 at 3:58
add a comment |
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
5
down vote
accepted
The implementation is like that because the javadoc states it should be:
Implementation Requirements:
The default implementation is equivalent to, for this map:
V v = map.get(key);
if (v == null)
v = map.put(key, value);
return v;
But why is it specified like that? Probably to get reasonably well-defined behavior in the presence of race conditions that cannot be addressed1.
First, here's what the javadoc says about it.
The default implementation makes no guarantees about synchronization or atomicity properties of this method. Any implementation providing atomicity guarantees must override this method and document its concurrency properties.
So how will the above implementation behave?
If there is no race with another thread mutating that map entry, it will either return
null
or the previous value (without updating the map).If there is a race and another thread updates the entry immediately after the
get
call, then themap.put
call will NOT returnnull
. Instead, it will actually return the value that the other thread inserted. And that value will be returned to the caller.
Note that this is not exactly in conformance with the primary description for the putIfAbsent
method. But this is excused by the "no guarantees" statement. And furthermore, it kind of makes sense.
Is this useful? Well ... probably yes, if the actual Map
method is thread safe. Because, if the calling code wanted to look, this would allow it to detect that a race had occurred and (if it made sense in the context of the overall application design) try to do something about it.
1 - The race conditions cannot be addressed in a default method which has no knowledge of the behavior of implementation classes. They could be addressed in the implementation classes themselves ... by overriding the method to have atomic properties, for example. And the method spec in the Map
javadocs clearly flag this possibility.
But the bottom line is that that supposedly redundant assignment is there because the spec clearly says that it should be there.
Wouldn't it be better design decision to change the linev = put(key, value);
toif (put(key, value) != null) throw new ConcurrentModificationException();
?
– John McClane
Nov 10 at 3:49
Nope. A concurrent collection doesn't throw that exception. And besides, as I explained, the specified behavior is predictable and useful. But it really doesn't alter anything debating if it could have been designed better.
– Stephen C
Nov 10 at 4:02
add a comment |
up vote
5
down vote
accepted
The implementation is like that because the javadoc states it should be:
Implementation Requirements:
The default implementation is equivalent to, for this map:
V v = map.get(key);
if (v == null)
v = map.put(key, value);
return v;
But why is it specified like that? Probably to get reasonably well-defined behavior in the presence of race conditions that cannot be addressed1.
First, here's what the javadoc says about it.
The default implementation makes no guarantees about synchronization or atomicity properties of this method. Any implementation providing atomicity guarantees must override this method and document its concurrency properties.
So how will the above implementation behave?
If there is no race with another thread mutating that map entry, it will either return
null
or the previous value (without updating the map).If there is a race and another thread updates the entry immediately after the
get
call, then themap.put
call will NOT returnnull
. Instead, it will actually return the value that the other thread inserted. And that value will be returned to the caller.
Note that this is not exactly in conformance with the primary description for the putIfAbsent
method. But this is excused by the "no guarantees" statement. And furthermore, it kind of makes sense.
Is this useful? Well ... probably yes, if the actual Map
method is thread safe. Because, if the calling code wanted to look, this would allow it to detect that a race had occurred and (if it made sense in the context of the overall application design) try to do something about it.
1 - The race conditions cannot be addressed in a default method which has no knowledge of the behavior of implementation classes. They could be addressed in the implementation classes themselves ... by overriding the method to have atomic properties, for example. And the method spec in the Map
javadocs clearly flag this possibility.
But the bottom line is that that supposedly redundant assignment is there because the spec clearly says that it should be there.
Wouldn't it be better design decision to change the linev = put(key, value);
toif (put(key, value) != null) throw new ConcurrentModificationException();
?
– John McClane
Nov 10 at 3:49
Nope. A concurrent collection doesn't throw that exception. And besides, as I explained, the specified behavior is predictable and useful. But it really doesn't alter anything debating if it could have been designed better.
– Stephen C
Nov 10 at 4:02
add a comment |
up vote
5
down vote
accepted
up vote
5
down vote
accepted
The implementation is like that because the javadoc states it should be:
Implementation Requirements:
The default implementation is equivalent to, for this map:
V v = map.get(key);
if (v == null)
v = map.put(key, value);
return v;
But why is it specified like that? Probably to get reasonably well-defined behavior in the presence of race conditions that cannot be addressed1.
First, here's what the javadoc says about it.
The default implementation makes no guarantees about synchronization or atomicity properties of this method. Any implementation providing atomicity guarantees must override this method and document its concurrency properties.
So how will the above implementation behave?
If there is no race with another thread mutating that map entry, it will either return
null
or the previous value (without updating the map).If there is a race and another thread updates the entry immediately after the
get
call, then themap.put
call will NOT returnnull
. Instead, it will actually return the value that the other thread inserted. And that value will be returned to the caller.
Note that this is not exactly in conformance with the primary description for the putIfAbsent
method. But this is excused by the "no guarantees" statement. And furthermore, it kind of makes sense.
Is this useful? Well ... probably yes, if the actual Map
method is thread safe. Because, if the calling code wanted to look, this would allow it to detect that a race had occurred and (if it made sense in the context of the overall application design) try to do something about it.
1 - The race conditions cannot be addressed in a default method which has no knowledge of the behavior of implementation classes. They could be addressed in the implementation classes themselves ... by overriding the method to have atomic properties, for example. And the method spec in the Map
javadocs clearly flag this possibility.
But the bottom line is that that supposedly redundant assignment is there because the spec clearly says that it should be there.
The implementation is like that because the javadoc states it should be:
Implementation Requirements:
The default implementation is equivalent to, for this map:
V v = map.get(key);
if (v == null)
v = map.put(key, value);
return v;
But why is it specified like that? Probably to get reasonably well-defined behavior in the presence of race conditions that cannot be addressed1.
First, here's what the javadoc says about it.
The default implementation makes no guarantees about synchronization or atomicity properties of this method. Any implementation providing atomicity guarantees must override this method and document its concurrency properties.
So how will the above implementation behave?
If there is no race with another thread mutating that map entry, it will either return
null
or the previous value (without updating the map).If there is a race and another thread updates the entry immediately after the
get
call, then themap.put
call will NOT returnnull
. Instead, it will actually return the value that the other thread inserted. And that value will be returned to the caller.
Note that this is not exactly in conformance with the primary description for the putIfAbsent
method. But this is excused by the "no guarantees" statement. And furthermore, it kind of makes sense.
Is this useful? Well ... probably yes, if the actual Map
method is thread safe. Because, if the calling code wanted to look, this would allow it to detect that a race had occurred and (if it made sense in the context of the overall application design) try to do something about it.
1 - The race conditions cannot be addressed in a default method which has no knowledge of the behavior of implementation classes. They could be addressed in the implementation classes themselves ... by overriding the method to have atomic properties, for example. And the method spec in the Map
javadocs clearly flag this possibility.
But the bottom line is that that supposedly redundant assignment is there because the spec clearly says that it should be there.
edited Nov 10 at 4:25
answered Nov 10 at 3:13
Stephen C
509k69556909
509k69556909
Wouldn't it be better design decision to change the linev = put(key, value);
toif (put(key, value) != null) throw new ConcurrentModificationException();
?
– John McClane
Nov 10 at 3:49
Nope. A concurrent collection doesn't throw that exception. And besides, as I explained, the specified behavior is predictable and useful. But it really doesn't alter anything debating if it could have been designed better.
– Stephen C
Nov 10 at 4:02
add a comment |
Wouldn't it be better design decision to change the linev = put(key, value);
toif (put(key, value) != null) throw new ConcurrentModificationException();
?
– John McClane
Nov 10 at 3:49
Nope. A concurrent collection doesn't throw that exception. And besides, as I explained, the specified behavior is predictable and useful. But it really doesn't alter anything debating if it could have been designed better.
– Stephen C
Nov 10 at 4:02
Wouldn't it be better design decision to change the line
v = put(key, value);
to if (put(key, value) != null) throw new ConcurrentModificationException();
?– John McClane
Nov 10 at 3:49
Wouldn't it be better design decision to change the line
v = put(key, value);
to if (put(key, value) != null) throw new ConcurrentModificationException();
?– John McClane
Nov 10 at 3:49
Nope. A concurrent collection doesn't throw that exception. And besides, as I explained, the specified behavior is predictable and useful. But it really doesn't alter anything debating if it could have been designed better.
– Stephen C
Nov 10 at 4:02
Nope. A concurrent collection doesn't throw that exception. And besides, as I explained, the specified behavior is predictable and useful. But it really doesn't alter anything debating if it could have been designed better.
– Stephen C
Nov 10 at 4:02
add a comment |
up vote
3
down vote
It must always return the old value.
If the map is accessed by multiple threads, the value returned by put
may differ from the value returned by get
, so assigning to v
ensures better behavior.
Of course, the code isn't great for handling multi-threaded access, so any valid thread-safe implementation overrides the method to do it better, but this is the best that a generic default implementation can achieve.
IMO this is a poor implementation decision. If the value fromput
is different fromget
, then the method still updates the key-value pair, and further, the return value is non-null, which normally indicates that theput
did not actually happen.
– flakes
Nov 10 at 3:10
1
It is not an implementation decision. It is a specification decision. See my answer.
– Stephen C
Nov 10 at 3:58
add a comment |
up vote
3
down vote
It must always return the old value.
If the map is accessed by multiple threads, the value returned by put
may differ from the value returned by get
, so assigning to v
ensures better behavior.
Of course, the code isn't great for handling multi-threaded access, so any valid thread-safe implementation overrides the method to do it better, but this is the best that a generic default implementation can achieve.
IMO this is a poor implementation decision. If the value fromput
is different fromget
, then the method still updates the key-value pair, and further, the return value is non-null, which normally indicates that theput
did not actually happen.
– flakes
Nov 10 at 3:10
1
It is not an implementation decision. It is a specification decision. See my answer.
– Stephen C
Nov 10 at 3:58
add a comment |
up vote
3
down vote
up vote
3
down vote
It must always return the old value.
If the map is accessed by multiple threads, the value returned by put
may differ from the value returned by get
, so assigning to v
ensures better behavior.
Of course, the code isn't great for handling multi-threaded access, so any valid thread-safe implementation overrides the method to do it better, but this is the best that a generic default implementation can achieve.
It must always return the old value.
If the map is accessed by multiple threads, the value returned by put
may differ from the value returned by get
, so assigning to v
ensures better behavior.
Of course, the code isn't great for handling multi-threaded access, so any valid thread-safe implementation overrides the method to do it better, but this is the best that a generic default implementation can achieve.
answered Nov 10 at 3:06
Andreas
73.8k456118
73.8k456118
IMO this is a poor implementation decision. If the value fromput
is different fromget
, then the method still updates the key-value pair, and further, the return value is non-null, which normally indicates that theput
did not actually happen.
– flakes
Nov 10 at 3:10
1
It is not an implementation decision. It is a specification decision. See my answer.
– Stephen C
Nov 10 at 3:58
add a comment |
IMO this is a poor implementation decision. If the value fromput
is different fromget
, then the method still updates the key-value pair, and further, the return value is non-null, which normally indicates that theput
did not actually happen.
– flakes
Nov 10 at 3:10
1
It is not an implementation decision. It is a specification decision. See my answer.
– Stephen C
Nov 10 at 3:58
IMO this is a poor implementation decision. If the value from
put
is different from get
, then the method still updates the key-value pair, and further, the return value is non-null, which normally indicates that the put
did not actually happen.– flakes
Nov 10 at 3:10
IMO this is a poor implementation decision. If the value from
put
is different from get
, then the method still updates the key-value pair, and further, the return value is non-null, which normally indicates that the put
did not actually happen.– flakes
Nov 10 at 3:10
1
1
It is not an implementation decision. It is a specification decision. See my answer.
– Stephen C
Nov 10 at 3:58
It is not an implementation decision. It is a specification decision. See my answer.
– Stephen C
Nov 10 at 3:58
add a comment |
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53235624%2fredundant-assignment-in-maps-putifabsent-implementation%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
My guess is that it accounts for possibility of a race condition. This method is not an atomic action. The key value association may have been updated in between the
get
andput
calls– flakes
Nov 10 at 3:04
I thought about concurrency over-cautions, but the phrase "The default implementation makes no guarantees about synchronization or atomicity properties of this method" in the documentation of
putIfAbsent
suggests otherwise. Maybe it is an attempt to be on the safe side, as mentioned in answers.– John McClane
Nov 10 at 3:22