Success when there is no cache objects

[aloncarmel]

Hey,
I cant understand why this is happening:

    var NodeCache = require('node-cache');

            var mycache = new NodeCache( { stdTTL: 100, checkperiod: 120 } );

    mycache.get("test", function( err, cachepage ){

       console.log(cachepage);
       console.log(err);

      if(!err){

        //Object is present in cache.
        console.log('VIEW: Page from cache');

        res.jsonp(cachepage);

it ALWAYS go into the if(!erro){ } when err is null and cachepage is {} .

i dont understand whats the issue,

[Theadd]

That's because when there is no hit on the cache, it doesn't return an error but an empty object as second parameter.

Use something like if (typeof cachepage['test'] === "undefined") { to check for hit/miss.

[shellscape]

I'll throw my hat in the ring here and argue that it should at least return null. Returning an empty object when there's nothing represented in the cache is a likely anti-pattern at best. Requiring the consumer to then check for non-existent keys, or something like _.isEmpty, is a little weird too.

returning null or undefined is pretty standard practice, no?

[brainsiq]

Have to say was thrown by the same issue as well.

[philmander]

Returning a truthy value on miss seems very strange. Any rationale behind this?

[shellscape]

@mpneuried can we please get a comment on this?

[smrchy]

As i recall the reason for this behaviour is that GET and MGET can be used in the same way.

If it was just for the GET you could return just the value, but with MGET where you supply n keys you need to return those keys (if found) in the object.

[mpneuried]

Hi there,

as my colleague @smrchy explained the answer format of the .get() method is designed to match the .mget() method. So that both have the same format.
As you can see this module is at least more than 3 years old, which means i wont design the API like this again and i agree completly with all of your statements.

So my proposal is to change the behaviour of the .get() method and increase the version to 2.x.x to mark it as a breaking change.

Would this be a solution?

[mpneuried]

I implemented the solution within the v2 branch.
Because one of my projects i cannot publish now and have to wait for the next live update.
But you can check out the code here https://github.com/tcs-de/nodecache/tree/v2

[philmander]

Could you just return null if the get() call is a miss rather than an error? Making this a non-truthy value would make using the api much more convenient/usable than checking for an instance of an error:

if(!mycache.get('foo')) { ... }

[callmewa]

meanwhile

if(cache.get(key)[key])

[callmewa]

Also I would like to suggest creating a new NPM project for the v2.. too many people have already downloaded V1 and this is a fundamental breaking change

[shellscape]

@mpneuried I agree with @philmander; returning an error object here is a non-optimal solution. Anything in the nodejs ecosystem that deals in ESOMETHING errors will throw an ESOMETHING error, but I'm not aware of a single component/module that returns an error object. It would make a lot more sense to return null, or at the least, undefined. I can't get on board with the choice to return an object that the user didn't ask for, appreciate the effort very much, but a poor choice imho.

[mpneuried]

Version 2.1.0 is now published.
Now you'll receive a undefined on a miss.