New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SummingWithHitsCache class to also track key hits. #410
Conversation
extends SummingCache[K, V](capacity)(sgv) { | ||
|
||
def putWithHits(m: Map[K, V]): (Int, Option[Map[K, V]]) = { | ||
val keyHits = m.keys.filter(cache.get(_).isDefined).size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m.keys.count(cache.contains)
Let's add some tests, but otherwise this looks good. What do you think about the private constructor? That was unusual before and caused a problem here. Should we just make the constructor public? What do we gain from private constructor? (I did it before, but I think it was a mistake now). |
I actually don't know the reason of why it's private in the first place. So I just changed it to protected to make this work. If you feel the private accessor is not needed any more, I think let's go ahead change it to public. |
@johnynek Added several tests for it. |
/** | ||
* A SummingCache that also tracks the number of key hits | ||
*/ | ||
class SummingWithHitsCache[K, V] private (capacity: Int)(implicit sgv: Semigroup[V]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make this one public too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
Just out of curiosity, what's the original reason to make the constructor private?
after we fix the public issue, LGTM. |
Add SummingWithHitsCache class to also track key hits.
No description provided.