Return to Revisions

Your code is thread-safe as it is. [But not really... see the comments below.]

• Be careful with method and variable names. TempScheduler does not really have anything temporary. callServiceURL does a lot more than calling a URL. blockHost(hostname) would be better as addToBlockedHosts(hostname). call is non-descriptive. ClientData only contains information related to host names.

• Try to have each method doing only one thing. For example, callServiceURL should just return some json. Then parseResponse should take that json and return a list of host names. Finally, some other functions should add the host names to the set of blocked hosts.

• volatile has the same effect as AtomicReference and makes the code simpler. See this post.

• Instead of either AtomicReference or volatile you could just have a final ConcurrentHashMap which you would clear when you receive a new list.

• Your non-class ClientData with only static methods makes me cringe. Please make it a normal class. Create an instance of ClientData in the main thread and pass it to the secondary thread when you create it.