jmdobry / CacheFactory

CacheFactory is a very simple and useful synchronous key-value store for the browser.
http://www.pseudobry.com/CacheFactory
MIT License
30 stars 18 forks source link

feat(CacheFactory): new dispose method #5

Open ccrowhurstram opened 9 years ago

ccrowhurstram commented 9 years ago

Calling cache.dispose will dispose of that cache removing it from the CacheFactory; any items that the cache has added to underlying storage will not be removed

jmdobry commented 9 years ago

Can you please explain your motivation and use-case behind this pull request? I can read the code, but I can't read your mind ;)

Also, please remove your changes to any dist/ files from the PR.

ccrowhurstram commented 9 years ago

Hi Jason ,

Here's my specific use case.

I'm using localStorage to persist cache items between browser sessions. My application keeps track of each cache created by persisting the cache id in a list.

I have a screen / page that allows this list of caches to be administered. Whenever a cache needs to be administered, this screen needs to create an instance of a cache using the id from the list. But this admin screen does not know exactly the options that were used to create the cache, nor should it care as it's only responsible for exposing admin related functionality.

It creates the cache using sensible safe defaults. It uses the cache to perform the admin function - for example removing expired items - and then immediately disposes of the cache.

By disposing of the cache, the rest of the application code can create the cache with the real cache options. This is only possible if the cache can be removed from the CacheFactory but not destroyed

ccrowhurstram commented 9 years ago

I've removed the dist/ files from the PR

jmdobry commented 9 years ago

Why can't the admin page just use the cache created by the application?

ccrowhurstram commented 9 years ago

The application might not yet have [re]created the cache when the admin page is accessed.

Just to clarify - cache(s) will likely have been created in a previous browser session. These caches are lazy instantiated by the application in the next browser session only when the user happens to browse to a particular area of the application. For examples sake lets say one such area is named the invoicing module.

The invoicing module is responsible for knowing the options required for that cache and maybe will even decorate the cache with extra functionality.

Yet the admin page is available to administer the caches even before the caches have been lazy instantiated.

Of course, if the cache has already been created by the invoices module then the admin page will simply acquire a reference to it. But if the cache is not yet created it will need to instantiate the cache itself.

It will create the cache with options that might not necessarily be the ones that the invoicing module will use. It certainly won't know about the decorations that the invoicing module might want to apply to the cache. Therefore any cache the admin page creates needs to be immediately disposed of and references to it thrown away.

C

jmdobry commented 8 years ago

What you've implemented seems like a code smell to me. You have multiple places where you're creating the same object, which could be error prone because the multiple places may or may not use the same config to create the cache. Also, if you need to change the config you have to change it multiple places (Shotgun Surgery).

Each one of these caches is essentially a singleton. It shouldn't matter when or what accesses it, because when the cache is accessed, the same cache should be available. So, whether your invoice module needs the cache or admin module needs the cache, when they ask for it, they should get the same cache. So your cache creation code need to be moved to a single location where the cache instance can be created before anything needs it. Often in the singleton pattern, the instance is lazy-instantiated, which is okay. The key is that there is only a single point of instantiation.