Is it Thread-safe on this operating on ConcurrentHashMap?

Question

private final ConcurrentHashMap<Float, VoteItem> datum = new ConcurrentHashMap<>();

public void vote(float graduation) {
    datum.putIfAbsent(graduation, new VoteItem(graduation, new AtomicInteger(0)));
    datum.get(graduation).getNum().incrementAndGet();
}

Does the method vote is totally thread safe? VoteItem.getNum() returns an AtomicInteger? Or if there is a better way to achieve it?

Answer 1

If VoteItem#getNum() is thread-safe, e. g. returns final property, and no deletions are performed in parallel thread, your code is also thread-safe as there is no chance for putIfAbsent() to overwrite existing entry, and thus no chance for get() to return entry that is overwritten.

But there is more common way to achieve it using result of putIfAbsent(), which returns existing value if it is present for a given key:

public void vote(float graduation) {
    VoteItem i = datum.putIfAbsent(graduation, new VoteItem(graduation, new AtomicInteger(1)));
    if (i != null)
        i.getNum().incrementAndGet();
}

This handles possibility of concurrent removals as well. In contrast to your code, where concurrent removal can be performed between putIfAbsent() and get() thus causing NPE, here no such situation can occur.

And consider to use computeIfAbsent() instead of putIfAbsent() in order to avoid unnessessary VoteItem creations:

public void vote(float graduation) {
    datum.computeIfAbsent(graduation, g -> new VoteItem(g, new AtomicInteger(0)))
         .getNum()
         .incrementAndGet();
}

Calling getNum() on result is possible because in contrast to putIfAbsent(), which returns null if value didn't exist prior to insertion, it returns just computed value.