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

replace usize with NonZeroUsize for LRU's capacity #150

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

Yosi-Hezi
Copy link
Contributor

@Yosi-Hezi Yosi-Hezi commented Aug 16, 2022

There is no use for LRU with zero capacity. Using NonZeroUsize for the cap field ensures that the state of zero capacity is unreachable and simplifies the logic.

Pros:

  • Simplified logic, where zero capacity is avoided. A bit easier to extend the LRU functionality.
  • Improved and simplified get_or_insert API (no need to return Option)

Cons:

  • Additional use statements to doc.
  • Breaks previous API that uses the cap field

@Yosi-Hezi Yosi-Hezi mentioned this pull request Aug 17, 2022
Copy link
Owner

@jeromefroe jeromefroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, @Yosi-Hezi!

@jeromefroe jeromefroe merged commit b491112 into jeromefroe:master Aug 19, 2022
@behzadnouri
Copy link

Can you please consider reverting this commit?
Please see #165

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

Successfully merging this pull request may close these issues.

3 participants