Skip to content
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

Has method includes expired keys #147

Closed
hongkuancn opened this issue Aug 23, 2024 · 1 comment · Fixed by #148
Closed

Has method includes expired keys #147

hongkuancn opened this issue Aug 23, 2024 · 1 comment · Fixed by #148
Labels

Comments

@hongkuancn
Copy link
Contributor

I noticed the Has method will print true when checking an expired key

func Test_Has(t *testing.T) {
	cache := New[int, int]()
	cache.Set(1, 1, time.Nanosecond)
	time.Sleep(time.Second)
	fmt.Println(cache.Has(1))
}

output:

true

I think it makes more sense to exclude expired keys here and it won't cause much overhead just to check if the item expires. If so, I have some time to make the change.

@swithek
Copy link
Contributor

swithek commented Aug 23, 2024

I believe most data/status retrieval methods include expired keys (i.e., Len(), Keys(), Range(), Has()), so we should probably update all of them. Using item.isExpiredUnsafe() in them, just like you did in your latest PR, should be enough.

If you can tackle this issue, feel free to do that, I'll review your PRs ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants