CODEWITHSUNDEEP
javaconcurrencythread-safety
krackmoe
krackmoe

Reputation: 1763

ConcurrentHashmap does need synchronization in my implementation?

I am having some troubles by seeing if something needs to be synchronized or not, i will show you my sample solution. Do i actually need my methods synchronized? Or would there be a better approach? Because ConcurrentHashMap is thread safe by itself, the question arises because i have two of them, then i think i should synchronize the methods to not get the two maps out of sync, right?

The BookService is annotated with @Service, which implies it beeing a singleton.

@Service
public class BookService {

    private final ConcurrentMap<String, UUID> codeToUUIDs;
    private final ConcurrentMap<Book, String> bookToCodes;

    public BookService() {
        this.bookToCodes = fillBookCodes();
        this.codeToUUIDs = new ConcurrentHashMap<>();
    }

    public synchronized UUID add(Book book) {
        final String randomCode = bookToCodes.putIfAbsent(book, "randomCode");
        return codeToUUIDs.get(randomCode);
    }

    public synchronized UUID get(String code) {
        return codeToUUIDs.get(code);
    }
}

Upvotes: 0

Views: 297

Answers (2)

davidxxx
davidxxx

Reputation: 131456

The access to the map have to be synchronized only if you have a race condition that may create a not desirable inconsistency in the map content.
It may the case even by using a single map.

Actually you don't have any race condition in the get() and the add() methods. So synchronization is helpless.

suppose the add() method changed the state of one map according to what you put in the other, you would have a race condition and in this case, external synchronization would be required.

Upvotes: 1

vlsergey
vlsergey

Reputation: 264

Since both of maps need to be in sync, you need to use synchronized methods.

But since your methods are already synchronized, you don't need ConcurrentMaps here. Usual HashMaps are okay.

Upvotes: 4

Related Questions