On 04/11/2019 15:09, Daniel Fuchs wrote:
:
It looks like Cache#remove can be fixed simply by using an explicit
iterator and using iterator#remove instead of List#remove.
Yes - we could have used an Iterator. I don't like much LinkedList.
We could have used ArrayList and an explicit Iterator instead.
I suggested using CopyOnWriteArrayList just to avoid using the
Iterator explicitly. Cache are usually few writes and multiple
reads so that looked appropriate.
Would you still advised using plain ArrayList and an Iterator instead?
I think I agree Martin in that it looks strange to use COWAL for the
entries when all methods are synchronized. What are the access patterns
on a Cache instance? I assume mostly get methods, in which case allowing
concurrent access may be benefical. If there are a lot of calls to
remove then changing it ArrayList could be O(n) due to the shifting.
Maybe the simplest is to fix the issue at end by using the
Iterator::remove and then re-visit the Cache implementation later (you
mentioned other auth schemes so I assume it will need to be re-visited
at some point anyway).
-Alan.
