search

contain-rs / lru-cache

A cache that holds a limited number of key-value pairs
https://contain-rs.github.io/lru-cache/lru_cache/
Apache License 2.0
83 stars 36 forks source link

immutable get() #37

Closed bbigras closed 5 years ago

FlashCat commented 7 years ago

Thanks for the pull request, and welcome! The contain-rs team is excited to review your changes, and you should hear from @reem (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

apasel422 commented 7 years ago

Thanks for the PR! I'm not sure about the naming here, as we might want to better indicate that calling this method does /not/ affect the LRU order. In any case, could you add a note about the LRU behavior to the doc comment?

bbigras commented 7 years ago

we might want to better indicate that calling this method does /not/ affect the LRU order.

Yes. Something like get_without_lru or maybe get_unoptimal so people wouldn't use it without reading the doc?

In any case, could you add a note about the LRU behavior to the doc comment?

Do you mean the normal LRU behavior or just in the case of the new method?

g2p commented 7 years ago

You could call it peek(). Most libraries do.

In any case, I would find this feature useful.

compressed commented 7 years ago

Just add one anecdote: I was also looking for this get() or peek() immutable borrow function today as well. Would like to see this merged!

bbigras commented 7 years ago

You could call it peek(). Most libraries do.

I renamed it to peek() and rebased. Sorry for the delay.

g2p commented 7 years ago

@apasel422, what do you think?

garyyu commented 5 years ago

Why this PR doesn't merged? In case of a lot of reading request, the immutable get method will help for performance.

BTW, Thanks for this nice crate! 👍

bbigras commented 5 years ago

The project is in maintenance mode so I guess I should close this.