16

This is keySet() method on HashMap class from JDK. Why did the author assign the field keySet to local variable ks?

public Set<K> keySet() {
    Set<K> ks;
    return (ks = keySet) == null ? (keySet = new KeySet()) : ks;
}

What is the difference between the above and the below? Does this have something to do with thread-safety?

public Set<K> keySet() {
    return (keySet == null ? (keySet = new KeySet()) : keySet;
}
Oleksandr Pyrohov
  • 14,685
  • 6
  • 61
  • 90
joohwan
  • 191
  • 6
  • Maybe; though it also looks like he's trying to set a class level variable while ensuring that variable is not null and returning that value. I've not a clue beyond that though. I'd need to look at the whole class. – Richard Barker Jun 22 '16 at 02:11
  • Perhaps for clarity of intention – Sajeev Jun 22 '16 at 02:19
  • 1
    Could be considered as a duplicate of http://stackoverflow.com/q/28975415/3182664 http://stackoverflow.com/q/2785964/3182664 http://stackoverflow.com/q/37776179/3182664 http://stackoverflow.com/q/32619269/3182664 and likely some others.... – Marco13 Jun 23 '16 at 00:55

2 Answers2

9

If you look at the keySet declaration in the abstract class AbstractMap<K,V>, you will see that it is defined as:

transient volatile Set<K>  keySet;

Since it is volatile, reading it just once by using the local variable assignment is cheaper than reading it twice as would be in the other example you provided.

Furthermore, if you were to return the keySet variable directly, then all the client code would be dealing with a volatile reference vs. a non-volatile reference (i.e. the Set<K> ks)

Michael Markidis
  • 4,163
  • 1
  • 14
  • 21
  • 2
    Thanks for the answer about volatile field read, I haven't thought about it. Could you please explain "dealing with a volatile reference vs. a non-volatile reference" part ? – joohwan Jun 22 '16 at 04:07
  • 1
    There's no such notion as a "volatile reference vs. a non-volatile reference". – Boann Jun 23 '16 at 00:44
8

To expand slightly on Michael's answer, I expect it is there to ensure that the keySet() method never returns null, possibly in addition to providing performance benefits noted.

Given this code:

public Set<K> keySet() {
    return (keySet == null ? (keySet = new KeySet()) : keySet;
}

It would be at least theoretically possible in multi-threaded code that the keySet field could be set to null between the first read (keySet == null) and the second read, where it is returned. I haven't looked at the rest of the code, but I assume there are other places where keySet is potentially assigned null. Whether this is as a result of an issue seen in the wild, or a defensive measure would be a question for the authors.

The actual code:

public Set<K> keySet() {
    Set<K> ks;
    return (ks = keySet) == null ? (keySet = new KeySet()) : ks;
}

...doesn't have this problem as the field is only read once.

clstrfsck
  • 14,715
  • 4
  • 44
  • 59