When reading the source code, I stumbled upon this method in the JDK sources. Please note the declaration and initialization of v
and newValue
. We have here 'nice' undefined values, assignment in comparisons, 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 to writing code the above way instead of the 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 (besides showing off Java constructs), rather than going with the 'easy' way?
v
wasn't local it could prevent race conditions wherev
is changed between setting and reading. But here the only advantage seems to scare off people from touching the code :-)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.