search

caiogondim / fast-memoize.js

:rabbit2: Fastest possible memoization library
https://npm.im/fast-memoize
MIT License
2.58k stars 86 forks source link

.has() method for custom cache is never called. Should be removed from documentation? #77

Open uazure opened 5 years ago

uazure commented 5 years ago

I tried to use this memoizer for fuction with 5 arguments, and I implemented the custom cache with .get() .set() and .has() methods

We have 100% test coverage rule for our code. And after running tests I see that .has() method is never called. I checked the source code for fast-memoize and I see no usage of .has() method at all.

So, either documentation should be updated to not mention the .has() method as it is not required, or the code was overoptimized and instead of calling .has() and then .get() you assume that if .get() returns undefined, it means that it wasn't stored. If that's the case, please add this to documentation to manage expectations of library users better.

caiogondim commented 5 years ago

Hi there... 👋 Could you share the code you are using so I can better investigate?

caiogondim commented 5 years ago

I remember we at some point removed has call for speed purposes. If we used has, we would need to call has and get in order to get a value from cache. Nowadays we use only get, removing the need to call has. One call only to check and get values from cache.

We might change this in the future, so I don't want to change the docs.