svendiedrichsen / jollyday

Jollyday - A holiday API
Other
189 stars 114 forks source link

Avoid locking and use ConcurrentHashMap.computeIfAbsent(...) combination #67

Closed guidomedina closed 6 years ago

guidomedina commented 6 years ago

Avoid locking and use ConcurrentHashMap.computeIfAbsent(...) combination in the Cache.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.3%) to 82.412% when pulling 426d2c01e418dc1ab7bc266dee22959dc0ef7519 on guidomedina:master into 7a2133b05b702907b8302f5a6b4ac3433501172f on svendiedrichsen:master.

svendiedrichsen commented 6 years ago

Very good point. Please see my one single remark. Thank you very much.

guidomedina commented 6 years ago

Hi @svendiedrichsen what's the remark? I have been looking but I'm clueless, maybe it is a comment you meant to write but haven't?

guidomedina commented 6 years ago

I'm thinking maybe the lambda function which is generated from the parameter is sub-optimal, I'll see if I can refactor the ValueHandler so that it is similar to something I did for another project.

My idea is to avoid Lambdas being generated, making the loading function more static, something similar to: https://github.com/quickfix-j/quickfixj/blob/master/quickfixj-core/src/main/java/org/quickfixj/SimpleCache.java

svendiedrichsen commented 6 years ago

I have added a review comment with the code. You should be able to see it above or when you look at the changed files. Its about using the Map interface instead of the ConcurrentMap interface for the member.

guidomedina commented 6 years ago

Ah, didn't see it, I will fix tomorrow, I might redo that part in a better way, I'll get back to you tomorrow when I'm back at work.

guidomedina commented 6 years ago

OK, I managed to fix it from home.