facundoolano / redis-lru

Redis-backed LRU cache for Node.js
96 stars 13 forks source link

[changed] Set promise return the Key instead of the Value #16

Closed roonie007 closed 5 years ago

facundoolano commented 5 years ago

What's the rationale behind the change?

roonie007 commented 5 years ago

Check the issue #15 , As I said in the issue, if someone wants to access the Key throught the chaining Promise, he can't while with the change you can have access both the Key and the Value, here is an example :

Let's say you wanna create an API layer over this Module, and you want the keys to be generated on this layer side like this

const sha1File = (filepath) => {
  return hasha.fromFileSync(filepath);
}

const sha1String = (str) => {
  return hasha(str);
}

const cache = {
  get: {
    func_1(originalText) {
      return lruCache.get(sha1String(originalText));
    },
    func_2(filePath) {
      return lruCache.get(sha1File(filePath));
    }
  },
  set: {
    func_1(originalText, translatedText) {
      return lruCache.set(sha1String(originalText), translatedText);
    },
    func_2(filePath, extractedTextFromAudioFile) {
      return new Promise((resolve, reject) => {
        const key = sha1File(filePath);
        lruCache.set(key, extractedTextFromAudioFile).then(() => {
          resolve(key);
        }).catch(reject);
      });
    }
  }
}

as you see in the cache.set.func_2 to be able to use the Key on the Promise chaining, you have to create a promise to be able to access the Key, with the changes I made, you can access the key which will allow to access the value too.

roonie007 commented 5 years ago

Any news about this ?

facundoolano commented 5 years ago

I'm not convinced about the usefulness of this change. Your example can be written as

func_2(filePath, extractedTextFromAudioFile) {
  const key = sha1File(filePath);
  return lruCache.set(key, extractedTextFromAudioFile).then(() => key);
}