Redundant assignment in Map's putIfAbsent implementation









up vote
2
down vote

favorite
1












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.










share|improve this question

















  • 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










  • 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














up vote
2
down vote

favorite
1












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.










share|improve this question

















  • 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










  • 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












up vote
2
down vote

favorite
1









up vote
2
down vote

favorite
1






1





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.










share|improve this question













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






share|improve this question













share|improve this question











share|improve this question




share|improve this question










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












  • 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










  • 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







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












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?



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


  2. If there is a race and another thread updates the entry immediately after the get call, then the map.put call will NOT return null. 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.






share|improve this answer






















  • 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


















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.






share|improve this answer




















  • 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




    It is not an implementation decision. It is a specification decision. See my answer.
    – Stephen C
    Nov 10 at 3:58










Your Answer






StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "1"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);













draft saved

draft discarded


















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

























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?



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


  2. If there is a race and another thread updates the entry immediately after the get call, then the map.put call will NOT return null. 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.






share|improve this answer






















  • 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















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?



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


  2. If there is a race and another thread updates the entry immediately after the get call, then the map.put call will NOT return null. 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.






share|improve this answer






















  • 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













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?



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


  2. If there is a race and another thread updates the entry immediately after the get call, then the map.put call will NOT return null. 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.






share|improve this answer














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?



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


  2. If there is a race and another thread updates the entry immediately after the get call, then the map.put call will NOT return null. 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.







share|improve this answer














share|improve this answer



share|improve this answer








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

















  • 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
















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













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.






share|improve this answer




















  • 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




    It is not an implementation decision. It is a specification decision. See my answer.
    – Stephen C
    Nov 10 at 3:58














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.






share|improve this answer




















  • 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




    It is not an implementation decision. It is a specification decision. See my answer.
    – Stephen C
    Nov 10 at 3:58












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.






share|improve this answer












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.







share|improve this answer












share|improve this answer



share|improve this answer










answered Nov 10 at 3:06









Andreas

73.8k456118




73.8k456118











  • 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




    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







  • 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

















draft saved

draft discarded
















































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.




draft saved


draft discarded














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





















































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







Popular posts from this blog

How to how show current date and time by default on contact form 7 in WordPress without taking input from user in datetimepicker

Syphilis

Darth Vader #20