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

Fix confusing conventions in memcache client docs #20

Open
jvivs opened this issue Dec 8, 2015 · 5 comments
Open

Fix confusing conventions in memcache client docs #20

jvivs opened this issue Dec 8, 2015 · 5 comments

Comments

@jvivs
Copy link

jvivs commented Dec 8, 2015

Cache vs cached is confusing, easily leading astray someone familiar with TitleCaseForClasses.

Skimming the docs, it is easy to assume Cache is an object constructor, not an instance of an object with a handle to a configured cache client.

@jkrems
Copy link
Contributor

jkrems commented Dec 9, 2015

Hi! Thanks for bringing this up. Always happy to make the docs easier to understand. Would Cache::set(key, value, opts, cb) -> Promise[Value] be more obvious?

@jvivs
Copy link
Author

jvivs commented Dec 9, 2015

I'd suggest avoiding an uppercase for Cache unless it is actually a
class-like thing.

It appears that cached is a factory method for creating new instances of
Cache, correct? If that's the case, cacheClient.set(key, value, opts, cb) -> Promise[Value] seems like it would communicate what's actually
going on.

Happy to open a PR w/ a docs update if you'd like.

On Tue, Dec 8, 2015 at 7:00 PM, Jan Olaf Krems [email protected]
wrote:

Hi! Thanks for bringing this up. Always happy to make the docs easier to
understand. Would Cache::set(key, value, opts, cb) -> Promise[Value] be
more obvious?


Reply to this email directly or view it on GitHub
#20 (comment).

@jkrems
Copy link
Contributor

jkrems commented Dec 9, 2015

But then there's a mismatch between -> Promise[Value]/-> Cache and cacheClient. E.g. there's no 1:1 mapping that can tell the reader that cacheClient is an instance of Cache (which it is).

P.S.: I made the suggestion of Cache::foo because :: is a somewhat common way to denote "prototype method foo of the constructor Cache".

@jvivs
Copy link
Author

jvivs commented Dec 9, 2015

It seems the thing returned by cached() that you actually interact with
is a layer of abstraction--sometimes its an in-memory cache and sometimes
its a memcache pool running elsewhere.

The fact that cached can give you a unified api to both an in-memory
Cache and to a memcache pool is really nice because you don't have to
think about the particulars of managing cache with a given backend. But,
when you're interacting with the object returned by cached, you aren't
guaranteed to have a handle on the cache itself. Otherwise it seems like
this should delete the cache contents when configured against a pool:

var myCache = cached({/* memcache pool configuration */});
delete myCache;

😁

edit: I have no clue why GFM is failing me in this comment. Apologies for the code blocks!

@jkrems
Copy link
Contributor

jkrems commented Dec 9, 2015

Not sure I follow. Cache is the only interface that cached provides. It's always the same class, no matter what the backend is. You'll never get the actual memcached client. myCache in your example is always an instance of Cache.

I'm not sure what delete myCache is supposed to mean. If we ever add a myCache.flush(), then yes: I'd expect that to clear out the data from a memcached backend just like it would for an in-memory cache. Setting myCache = null will delete neither memcached nor in-memory data since cached will retain it.

But I'm not sure what the location of the cached data has to do with the return type docs..? Your initial point was "people could think Cache is a constructor". Which it is. Which is why I'm still a bit confused. My first guess was that you meant "Cache.get could be interpreted as a static method on the constructor". But maybe that's not what you're talking about?

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

No branches or pull requests

2 participants