Mixing
why to use assignment in comparison?
Clash Royale CLAN TAG#URR8PPP
up vote
6
down vote
favorite
When reading source code, I stumbled on this method in JDK sources. Please note declaration and initialization of v and newValue. We have here 'nice' undefined values, assignment in comparison, which is 'great', and extra brackets for worse readability. And other code smells.
default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
V v;
if ((v = get(key)) == null)
V newValue;
if ((newValue = mappingFunction.apply(key)) != null)
put(key, newValue);
return newValue;
return v;
But why? Is there any actual benefit of writing code as above instead of simple(ideally with negated v comparison):
default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
V v = get(key);
if (v == null)
V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
is there any actual benefit I'm not aware of(aside of flexing java constructs), than going with 'easy' way?
java
asked 1 hour ago
user1534084
818
 |Â
show 1 more comment
up vote
6
down vote
favorite
When reading source code, I stumbled on this method in JDK sources. Please note declaration and initialization of v and newValue. We have here 'nice' undefined values, assignment in comparison, which is 'great', and extra brackets for worse readability. And other code smells.
default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
V v;
if ((v = get(key)) == null)
V newValue;
if ((newValue = mappingFunction.apply(key)) != null)
put(key, newValue);
return newValue;
return v;
But why? Is there any actual benefit of writing code as above instead of simple(ideally with negated v comparison):
default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
V v = get(key);
if (v == null)
V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
is there any actual benefit I'm not aware of(aside of flexing java constructs), than going with 'easy' way?
java
asked 1 hour ago
user1534084
818
3
Ifvwasn't local it could prevent race conditions wherevis changed between setting and reading. But here the only advantage seems to scare off people from touching the code :-)
– Michael Butscher
1 hour ago
This is fromConcurrentMap, right? You might want to link to the exact source and version to make this clear. I've seen this sort of thing in many JDK classes; it's probably idiomatic to the author but doesn't really obey strict style guides. Anyways, a relevant question.
– Mick Mnemonic
1 hour ago
5
Purely difference of opinion when it comes to style.
– Joe C
1 hour ago
2
It's just a peculiar code style (Probably Doug Lea's? It would break his own conventions: "Avoid assignments (``='') inside if and while conditions. "). Would not recommend this in your own code for exactly the reasons you outlined.
– Petr JaneÄÂek
1 hour ago
Mick Mnemonic: it's just in Map. java.util.Map#computeIfAbsent I do like more readable code, indeed, but I don't think JDK authors are out of their minds, thus I was asking, why they opted for seemingly suboptimal expression.
– user1534084
59 mins ago
 |Â
show 1 more comment
up vote
6
down vote
favorite
up vote
6
down vote
favorite
When reading source code, I stumbled on this method in JDK sources. Please note declaration and initialization of v and newValue. We have here 'nice' undefined values, assignment in comparison, which is 'great', and extra brackets for worse readability. And other code smells.
default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
V v;
if ((v = get(key)) == null)
V newValue;
if ((newValue = mappingFunction.apply(key)) != null)
put(key, newValue);
return newValue;
return v;
But why? Is there any actual benefit of writing code as above instead of simple(ideally with negated v comparison):
default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
V v = get(key);
if (v == null)
V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
is there any actual benefit I'm not aware of(aside of flexing java constructs), than going with 'easy' way?
java
asked 1 hour ago
user1534084
818
When reading source code, I stumbled on this method in JDK sources. Please note declaration and initialization of v and newValue. We have here 'nice' undefined values, assignment in comparison, which is 'great', and extra brackets for worse readability. And other code smells.
default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
V v;
if ((v = get(key)) == null)
V newValue;
if ((newValue = mappingFunction.apply(key)) != null)
put(key, newValue);
return newValue;
return v;
But why? Is there any actual benefit of writing code as above instead of simple(ideally with negated v comparison):
default V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
V v = get(key);
if (v == null)
V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
is there any actual benefit I'm not aware of(aside of flexing java constructs), than going with 'easy' way?
java
java
asked 1 hour ago
user1534084
818
asked 1 hour ago
user1534084
818
asked 1 hour ago
user1534084
818
asked 1 hour ago
user1534084
818
asked 1 hour ago
user1534084
818
818
3
Ifvwasn't local it could prevent race conditions wherevis changed between setting and reading. But here the only advantage seems to scare off people from touching the code :-)
– Michael Butscher
1 hour ago
This is fromConcurrentMap, right? You might want to link to the exact source and version to make this clear. I've seen this sort of thing in many JDK classes; it's probably idiomatic to the author but doesn't really obey strict style guides. Anyways, a relevant question.
– Mick Mnemonic
1 hour ago
5
Purely difference of opinion when it comes to style.
– Joe C
1 hour ago
2
It's just a peculiar code style (Probably Doug Lea's? It would break his own conventions: "Avoid assignments (``='') inside if and while conditions. "). Would not recommend this in your own code for exactly the reasons you outlined.
– Petr JaneÄÂek
1 hour ago
Mick Mnemonic: it's just in Map. java.util.Map#computeIfAbsent I do like more readable code, indeed, but I don't think JDK authors are out of their minds, thus I was asking, why they opted for seemingly suboptimal expression.
– user1534084
59 mins ago
 |Â
show 1 more comment
3
Ifvwasn't local it could prevent race conditions wherevis changed between setting and reading. But here the only advantage seems to scare off people from touching the code :-)
– Michael Butscher
1 hour ago
This is fromConcurrentMap, right? You might want to link to the exact source and version to make this clear. I've seen this sort of thing in many JDK classes; it's probably idiomatic to the author but doesn't really obey strict style guides. Anyways, a relevant question.
– Mick Mnemonic
1 hour ago
5
Purely difference of opinion when it comes to style.
– Joe C
1 hour ago
2
It's just a peculiar code style (Probably Doug Lea's? It would break his own conventions: "Avoid assignments (``='') inside if and while conditions. "). Would not recommend this in your own code for exactly the reasons you outlined.
– Petr JaneÄÂek
1 hour ago
Mick Mnemonic: it's just in Map. java.util.Map#computeIfAbsent I do like more readable code, indeed, but I don't think JDK authors are out of their minds, thus I was asking, why they opted for seemingly suboptimal expression.
– user1534084
59 mins ago
3
3
If
v wasn't local it could prevent race conditions where v is changed between setting and reading. But here the only advantage seems to scare off people from touching the code :-)– Michael Butscher
1 hour ago
If
v wasn't local it could prevent race conditions where v is changed between setting and reading. But here the only advantage seems to scare off people from touching the code :-)– Michael Butscher
1 hour ago
This is from
ConcurrentMap, right? You might want to link to the exact source and version to make this clear. I've seen this sort of thing in many JDK classes; it's probably idiomatic to the author but doesn't really obey strict style guides. Anyways, a relevant question.– Mick Mnemonic
1 hour ago
This is from
ConcurrentMap, right? You might want to link to the exact source and version to make this clear. I've seen this sort of thing in many JDK classes; it's probably idiomatic to the author but doesn't really obey strict style guides. Anyways, a relevant question.– Mick Mnemonic
1 hour ago
5
5
Purely difference of opinion when it comes to style.
– Joe C
1 hour ago
Purely difference of opinion when it comes to style.
– Joe C
1 hour ago
2
2
It's just a peculiar code style (Probably Doug Lea's? It would break his own conventions: "Avoid assignments (``='') inside if and while conditions. "). Would not recommend this in your own code for exactly the reasons you outlined.
– Petr JaneÄÂek
1 hour ago
It's just a peculiar code style (Probably Doug Lea's? It would break his own conventions: "Avoid assignments (``='') inside if and while conditions. "). Would not recommend this in your own code for exactly the reasons you outlined.
– Petr JaneÄÂek
1 hour ago
Mick Mnemonic: it's just in Map. java.util.Map#computeIfAbsent I do like more readable code, indeed, but I don't think JDK authors are out of their minds, thus I was asking, why they opted for seemingly suboptimal expression.
– user1534084
59 mins ago
Mick Mnemonic: it's just in Map. java.util.Map#computeIfAbsent I do like more readable code, indeed, but I don't think JDK authors are out of their minds, thus I was asking, why they opted for seemingly suboptimal expression.
– user1534084
59 mins ago
 |Â
show 1 more comment
3 Answers
3
active
oldest
votes
up vote
3
down vote
accepted
#microoptimization (but in case of a standard library it could matter), and:
#inertia: this pattern was common among C programmers back in the 90-ies, so the titans of computer science may still use this style.
There's no point to write such code for new business logic, unless performance is really critical.
The (micro)optimization:
The bytecode produced by javac (JDK 11) for the original ("bad") version is one JVM-operation less than the (nicer) code. Why? The JDK's version "uses" the return value of the assignment operator (rather than loading the value from a variable) for the if condition evaluation.
However, this is more a limitation of javac's optimization possibilities than a reason to write the less-readable code.
Here's the bytecode for the JDK version, cited in the question:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: dup
11: astore_3
12: ifnonnull 39
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: dup
23: astore 4
25: ifnull 39
28: aload_0
29: aload_1
30: aload 4
32: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
35: pop
36: aload 4
38: areturn
39: aload_3
40: areturn
Below is the bytecode of a more readable version:
public V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
final V v = get(key);
if (v == null)
final V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
.. and the bytecode is:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: astore_3
11: aload_3
12: ifnonnull 40
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: astore 4
24: aload 4
26: ifnull 40
29: aload_0
30: aload_1
31: aload 4
33: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
36: pop
37: aload 4
39: areturn
40: aload_3
41: areturn
answered 1 hour ago
Alex Shesterov
14.2k83965
I believe this is the correct answer. I presume, that JDK devels cannot drive, well if anythihg at least shouldn't drive their work based on own preferences, but based on measured performance. And core libraries like collections has to be fast. Thus microoptimization makes sense here. My eyes still hurt a little from this code, but that's just my habbit what I'm used to look at. So again, I believe this is the correct answer. Thanks for helping me understand it!
– user1534084
50 mins ago
While this might be an optimization, I think shmosel's answer makes more sense. Only the author knows the real reason.
– Mick Mnemonic
44 mins ago
add a comment |Â
up vote
4
down vote
I agree: it's a code smell and I definitely prefer the second version.
The arcane use of assignment and comparison in a single expression can result in somewhat shorter code in case you have a loop, e.g.:
V v;
while ((v = getNext()) != null)
// ... do something with v ...
To remove the code smell, you need more code and you need to assign the variable in two places:
V v = getNext();
while (v != null)
// ...
v = getNext();
Or you need to move the loop exit condition after the assignment:
while (true)
V v = getNext();
if (v == null)
break;
// ...
For an if statement, it certainly makes no sense. And even for a loop, I would avoid it.
answered 1 hour ago
Codo
49.1k11109146
1
I wouldn't call it arcane by any means. Ubiquitous but not recommended perhaps?
– Mad Physicist
1 hour ago
It very much reminds me of old C code likeif (t = p->next)...which additionally depends on the implicit conversion of a non-null pointer to true / null pointer to false. But you're right: arcane is not the right word.
– Codo
1 hour ago
add a comment |Â
up vote
4
down vote
I think it's mostly a matter of preference; I'm under the impression Doug Lea prefers this terse style. But if you look at the original version of the method, it makes a little more sense, since it's paired with newValue which does need inline assignment:
default V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
V v, newValue;
return ((v = get(key)) == null &&
(newValue = mappingFunction.apply(key)) != null &&
(v = putIfAbsent(key, newValue)) == null) ? newValue : v;
answered 1 hour ago
shmosel
34.8k43689
add a comment |Â
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
3
down vote
accepted
#microoptimization (but in case of a standard library it could matter), and:
#inertia: this pattern was common among C programmers back in the 90-ies, so the titans of computer science may still use this style.
There's no point to write such code for new business logic, unless performance is really critical.
The (micro)optimization:
The bytecode produced by javac (JDK 11) for the original ("bad") version is one JVM-operation less than the (nicer) code. Why? The JDK's version "uses" the return value of the assignment operator (rather than loading the value from a variable) for the if condition evaluation.
However, this is more a limitation of javac's optimization possibilities than a reason to write the less-readable code.
Here's the bytecode for the JDK version, cited in the question:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: dup
11: astore_3
12: ifnonnull 39
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: dup
23: astore 4
25: ifnull 39
28: aload_0
29: aload_1
30: aload 4
32: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
35: pop
36: aload 4
38: areturn
39: aload_3
40: areturn
Below is the bytecode of a more readable version:
public V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
final V v = get(key);
if (v == null)
final V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
.. and the bytecode is:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: astore_3
11: aload_3
12: ifnonnull 40
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: astore 4
24: aload 4
26: ifnull 40
29: aload_0
30: aload_1
31: aload 4
33: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
36: pop
37: aload 4
39: areturn
40: aload_3
41: areturn
answered 1 hour ago
Alex Shesterov
14.2k83965
I believe this is the correct answer. I presume, that JDK devels cannot drive, well if anythihg at least shouldn't drive their work based on own preferences, but based on measured performance. And core libraries like collections has to be fast. Thus microoptimization makes sense here. My eyes still hurt a little from this code, but that's just my habbit what I'm used to look at. So again, I believe this is the correct answer. Thanks for helping me understand it!
– user1534084
50 mins ago
While this might be an optimization, I think shmosel's answer makes more sense. Only the author knows the real reason.
– Mick Mnemonic
44 mins ago
add a comment |Â
up vote
3
down vote
accepted
#microoptimization (but in case of a standard library it could matter), and:
#inertia: this pattern was common among C programmers back in the 90-ies, so the titans of computer science may still use this style.
There's no point to write such code for new business logic, unless performance is really critical.
The (micro)optimization:
The bytecode produced by javac (JDK 11) for the original ("bad") version is one JVM-operation less than the (nicer) code. Why? The JDK's version "uses" the return value of the assignment operator (rather than loading the value from a variable) for the if condition evaluation.
However, this is more a limitation of javac's optimization possibilities than a reason to write the less-readable code.
Here's the bytecode for the JDK version, cited in the question:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: dup
11: astore_3
12: ifnonnull 39
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: dup
23: astore 4
25: ifnull 39
28: aload_0
29: aload_1
30: aload 4
32: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
35: pop
36: aload 4
38: areturn
39: aload_3
40: areturn
Below is the bytecode of a more readable version:
public V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
final V v = get(key);
if (v == null)
final V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
.. and the bytecode is:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: astore_3
11: aload_3
12: ifnonnull 40
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: astore 4
24: aload 4
26: ifnull 40
29: aload_0
30: aload_1
31: aload 4
33: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
36: pop
37: aload 4
39: areturn
40: aload_3
41: areturn
answered 1 hour ago
Alex Shesterov
14.2k83965
I believe this is the correct answer. I presume, that JDK devels cannot drive, well if anythihg at least shouldn't drive their work based on own preferences, but based on measured performance. And core libraries like collections has to be fast. Thus microoptimization makes sense here. My eyes still hurt a little from this code, but that's just my habbit what I'm used to look at. So again, I believe this is the correct answer. Thanks for helping me understand it!
– user1534084
50 mins ago
While this might be an optimization, I think shmosel's answer makes more sense. Only the author knows the real reason.
– Mick Mnemonic
44 mins ago
add a comment |Â
up vote
3
down vote
accepted
up vote
3
down vote
accepted
#microoptimization (but in case of a standard library it could matter), and:
#inertia: this pattern was common among C programmers back in the 90-ies, so the titans of computer science may still use this style.
There's no point to write such code for new business logic, unless performance is really critical.
The (micro)optimization:
The bytecode produced by javac (JDK 11) for the original ("bad") version is one JVM-operation less than the (nicer) code. Why? The JDK's version "uses" the return value of the assignment operator (rather than loading the value from a variable) for the if condition evaluation.
However, this is more a limitation of javac's optimization possibilities than a reason to write the less-readable code.
Here's the bytecode for the JDK version, cited in the question:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: dup
11: astore_3
12: ifnonnull 39
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: dup
23: astore 4
25: ifnull 39
28: aload_0
29: aload_1
30: aload 4
32: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
35: pop
36: aload 4
38: areturn
39: aload_3
40: areturn
Below is the bytecode of a more readable version:
public V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
final V v = get(key);
if (v == null)
final V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
.. and the bytecode is:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: astore_3
11: aload_3
12: ifnonnull 40
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: astore 4
24: aload 4
26: ifnull 40
29: aload_0
30: aload_1
31: aload 4
33: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
36: pop
37: aload 4
39: areturn
40: aload_3
41: areturn
answered 1 hour ago
Alex Shesterov
14.2k83965
#microoptimization (but in case of a standard library it could matter), and:
#inertia: this pattern was common among C programmers back in the 90-ies, so the titans of computer science may still use this style.
There's no point to write such code for new business logic, unless performance is really critical.
The (micro)optimization:
The bytecode produced by javac (JDK 11) for the original ("bad") version is one JVM-operation less than the (nicer) code. Why? The JDK's version "uses" the return value of the assignment operator (rather than loading the value from a variable) for the if condition evaluation.
However, this is more a limitation of javac's optimization possibilities than a reason to write the less-readable code.
Here's the bytecode for the JDK version, cited in the question:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: dup
11: astore_3
12: ifnonnull 39
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: dup
23: astore 4
25: ifnull 39
28: aload_0
29: aload_1
30: aload 4
32: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
35: pop
36: aload 4
38: areturn
39: aload_3
40: areturn
Below is the bytecode of a more readable version:
public V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
Objects.requireNonNull(mappingFunction);
final V v = get(key);
if (v == null)
final V newValue = mappingFunction.apply(key);
if (newValue != null)
put(key, newValue);
return newValue;
return v;
.. and the bytecode is:
0: aload_2
1: invokestatic #2 // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
4: pop
5: aload_0
6: aload_1
7: invokevirtual #3 // Method get:(Ljava/lang/Object;)Ljava/lang/Object;
10: astore_3
11: aload_3
12: ifnonnull 40
15: aload_2
16: aload_1
17: invokeinterface #4, 2 // InterfaceMethod java/util/function/Function.apply:(Ljava/lang/Object;)Ljava/lang/Object;
22: astore 4
24: aload 4
26: ifnull 40
29: aload_0
30: aload_1
31: aload 4
33: invokevirtual #5 // Method put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
36: pop
37: aload 4
39: areturn
40: aload_3
41: areturn
answered 1 hour ago
Alex Shesterov
14.2k83965
edited 3 mins ago
answered 1 hour ago
Alex Shesterov
14.2k83965
answered 1 hour ago
Alex Shesterov
14.2k83965
answered 1 hour ago
Alex Shesterov
14.2k83965
14.2k83965
I believe this is the correct answer. I presume, that JDK devels cannot drive, well if anythihg at least shouldn't drive their work based on own preferences, but based on measured performance. And core libraries like collections has to be fast. Thus microoptimization makes sense here. My eyes still hurt a little from this code, but that's just my habbit what I'm used to look at. So again, I believe this is the correct answer. Thanks for helping me understand it!
– user1534084
50 mins ago
While this might be an optimization, I think shmosel's answer makes more sense. Only the author knows the real reason.
– Mick Mnemonic
44 mins ago
add a comment |Â
I believe this is the correct answer. I presume, that JDK devels cannot drive, well if anythihg at least shouldn't drive their work based on own preferences, but based on measured performance. And core libraries like collections has to be fast. Thus microoptimization makes sense here. My eyes still hurt a little from this code, but that's just my habbit what I'm used to look at. So again, I believe this is the correct answer. Thanks for helping me understand it!
– user1534084
50 mins ago
While this might be an optimization, I think shmosel's answer makes more sense. Only the author knows the real reason.
– Mick Mnemonic
44 mins ago
I believe this is the correct answer. I presume, that JDK devels cannot drive, well if anythihg at least shouldn't drive their work based on own preferences, but based on measured performance. And core libraries like collections has to be fast. Thus microoptimization makes sense here. My eyes still hurt a little from this code, but that's just my habbit what I'm used to look at. So again, I believe this is the correct answer. Thanks for helping me understand it!
– user1534084
50 mins ago
I believe this is the correct answer. I presume, that JDK devels cannot drive, well if anythihg at least shouldn't drive their work based on own preferences, but based on measured performance. And core libraries like collections has to be fast. Thus microoptimization makes sense here. My eyes still hurt a little from this code, but that's just my habbit what I'm used to look at. So again, I believe this is the correct answer. Thanks for helping me understand it!
– user1534084
50 mins ago
While this might be an optimization, I think shmosel's answer makes more sense. Only the author knows the real reason.
– Mick Mnemonic
44 mins ago
While this might be an optimization, I think shmosel's answer makes more sense. Only the author knows the real reason.
– Mick Mnemonic
44 mins ago
add a comment |Â
up vote
4
down vote
I agree: it's a code smell and I definitely prefer the second version.
The arcane use of assignment and comparison in a single expression can result in somewhat shorter code in case you have a loop, e.g.:
V v;
while ((v = getNext()) != null)
// ... do something with v ...
To remove the code smell, you need more code and you need to assign the variable in two places:
V v = getNext();
while (v != null)
// ...
v = getNext();
Or you need to move the loop exit condition after the assignment:
while (true)
V v = getNext();
if (v == null)
break;
// ...
For an if statement, it certainly makes no sense. And even for a loop, I would avoid it.
answered 1 hour ago
Codo
49.1k11109146
1
I wouldn't call it arcane by any means. Ubiquitous but not recommended perhaps?
– Mad Physicist
1 hour ago
It very much reminds me of old C code likeif (t = p->next)...which additionally depends on the implicit conversion of a non-null pointer to true / null pointer to false. But you're right: arcane is not the right word.
– Codo
1 hour ago
add a comment |Â
up vote
4
down vote
I agree: it's a code smell and I definitely prefer the second version.
The arcane use of assignment and comparison in a single expression can result in somewhat shorter code in case you have a loop, e.g.:
V v;
while ((v = getNext()) != null)
// ... do something with v ...
To remove the code smell, you need more code and you need to assign the variable in two places:
V v = getNext();
while (v != null)
// ...
v = getNext();
Or you need to move the loop exit condition after the assignment:
while (true)
V v = getNext();
if (v == null)
break;
// ...
For an if statement, it certainly makes no sense. And even for a loop, I would avoid it.
answered 1 hour ago
Codo
49.1k11109146
1
I wouldn't call it arcane by any means. Ubiquitous but not recommended perhaps?
– Mad Physicist
1 hour ago
It very much reminds me of old C code likeif (t = p->next)...which additionally depends on the implicit conversion of a non-null pointer to true / null pointer to false. But you're right: arcane is not the right word.
– Codo
1 hour ago
add a comment |Â
up vote
4
down vote
up vote
4
down vote
I agree: it's a code smell and I definitely prefer the second version.
The arcane use of assignment and comparison in a single expression can result in somewhat shorter code in case you have a loop, e.g.:
V v;
while ((v = getNext()) != null)
// ... do something with v ...
To remove the code smell, you need more code and you need to assign the variable in two places:
V v = getNext();
while (v != null)
// ...
v = getNext();
Or you need to move the loop exit condition after the assignment:
while (true)
V v = getNext();
if (v == null)
break;
// ...
For an if statement, it certainly makes no sense. And even for a loop, I would avoid it.
answered 1 hour ago
Codo
49.1k11109146
I agree: it's a code smell and I definitely prefer the second version.
The arcane use of assignment and comparison in a single expression can result in somewhat shorter code in case you have a loop, e.g.:
V v;
while ((v = getNext()) != null)
// ... do something with v ...
To remove the code smell, you need more code and you need to assign the variable in two places:
V v = getNext();
while (v != null)
// ...
v = getNext();
Or you need to move the loop exit condition after the assignment:
while (true)
V v = getNext();
if (v == null)
break;
// ...
For an if statement, it certainly makes no sense. And even for a loop, I would avoid it.
answered 1 hour ago
Codo
49.1k11109146
answered 1 hour ago
Codo
49.1k11109146
answered 1 hour ago
Codo
49.1k11109146
answered 1 hour ago
Codo
49.1k11109146
49.1k11109146
1
I wouldn't call it arcane by any means. Ubiquitous but not recommended perhaps?
– Mad Physicist
1 hour ago
It very much reminds me of old C code likeif (t = p->next)...which additionally depends on the implicit conversion of a non-null pointer to true / null pointer to false. But you're right: arcane is not the right word.
– Codo
1 hour ago
add a comment |Â
1
I wouldn't call it arcane by any means. Ubiquitous but not recommended perhaps?
– Mad Physicist
1 hour ago
It very much reminds me of old C code likeif (t = p->next)...which additionally depends on the implicit conversion of a non-null pointer to true / null pointer to false. But you're right: arcane is not the right word.
– Codo
1 hour ago
1
1
I wouldn't call it arcane by any means. Ubiquitous but not recommended perhaps?
– Mad Physicist
1 hour ago
I wouldn't call it arcane by any means. Ubiquitous but not recommended perhaps?
– Mad Physicist
1 hour ago
It very much reminds me of old C code like
if (t = p->next)... which additionally depends on the implicit conversion of a non-null pointer to true / null pointer to false. But you're right: arcane is not the right word.– Codo
1 hour ago
It very much reminds me of old C code like
if (t = p->next)... which additionally depends on the implicit conversion of a non-null pointer to true / null pointer to false. But you're right: arcane is not the right word.– Codo
1 hour ago
add a comment |Â
up vote
4
down vote
I think it's mostly a matter of preference; I'm under the impression Doug Lea prefers this terse style. But if you look at the original version of the method, it makes a little more sense, since it's paired with newValue which does need inline assignment:
default V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
V v, newValue;
return ((v = get(key)) == null &&
(newValue = mappingFunction.apply(key)) != null &&
(v = putIfAbsent(key, newValue)) == null) ? newValue : v;
answered 1 hour ago
shmosel
34.8k43689
add a comment |Â
up vote
4
down vote
I think it's mostly a matter of preference; I'm under the impression Doug Lea prefers this terse style. But if you look at the original version of the method, it makes a little more sense, since it's paired with newValue which does need inline assignment:
default V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
V v, newValue;
return ((v = get(key)) == null &&
(newValue = mappingFunction.apply(key)) != null &&
(v = putIfAbsent(key, newValue)) == null) ? newValue : v;
answered 1 hour ago
shmosel
34.8k43689
add a comment |Â
up vote
4
down vote
up vote
4
down vote
I think it's mostly a matter of preference; I'm under the impression Doug Lea prefers this terse style. But if you look at the original version of the method, it makes a little more sense, since it's paired with newValue which does need inline assignment:
default V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
V v, newValue;
return ((v = get(key)) == null &&
(newValue = mappingFunction.apply(key)) != null &&
(v = putIfAbsent(key, newValue)) == null) ? newValue : v;
answered 1 hour ago
shmosel
34.8k43689
I think it's mostly a matter of preference; I'm under the impression Doug Lea prefers this terse style. But if you look at the original version of the method, it makes a little more sense, since it's paired with newValue which does need inline assignment:
default V computeIfAbsent(K key,
Function<? super K, ? extends V> mappingFunction)
V v, newValue;
return ((v = get(key)) == null &&
(newValue = mappingFunction.apply(key)) != null &&
(v = putIfAbsent(key, newValue)) == null) ? newValue : v;
answered 1 hour ago
shmosel
34.8k43689
answered 1 hour ago
shmosel
34.8k43689
answered 1 hour ago
shmosel
34.8k43689
answered 1 hour ago
shmosel
34.8k43689
34.8k43689
add a comment |Â
add a comment |Â
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
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53016055%2fwhy-to-use-assignment-in-comparison%23new-answer', 'question_page');
);
Post as a guest
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
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
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
3
If
vwasn't local it could prevent race conditions wherevis changed between setting and reading. But here the only advantage seems to scare off people from touching the code :-)– Michael Butscher
1 hour ago
This is from
ConcurrentMap, right? You might want to link to the exact source and version to make this clear. I've seen this sort of thing in many JDK classes; it's probably idiomatic to the author but doesn't really obey strict style guides. Anyways, a relevant question.– Mick Mnemonic
1 hour ago
5
Purely difference of opinion when it comes to style.
– Joe C
1 hour ago
2
It's just a peculiar code style (Probably Doug Lea's? It would break his own conventions: "Avoid assignments (``='') inside if and while conditions. "). Would not recommend this in your own code for exactly the reasons you outlined.
– Petr JaneÄÂek
1 hour ago
Mick Mnemonic: it's just in Map. java.util.Map#computeIfAbsent I do like more readable code, indeed, but I don't think JDK authors are out of their minds, thus I was asking, why they opted for seemingly suboptimal expression.
– user1534084
59 mins ago