1
\$\begingroup\$

Here is a caching code of a nodejs / typescript app using lru Cache module from https://www.npmjs.com/package/lru-cache. For various reason i need to setup separate caches: one for homeCache another one for detailThreadCache... but this is just the beginning I may need to set up more.

This code is working, but how would you refactor it to improve code reuse ? Would you use object oriented paradigm ? If yes how ?

const homeCache = lruCache( 50 )
const detailThreadCache = lruCache( 150 )

export function homeCachedData( key ) {
    return homeCache.get( key )
}

export function homeSetCache( key: string, value, duration: number ) {
    homeCache.set( key, value, duration )
}

export function detailThreadGetCache( key ) {
    return detailThreadCache.get( key )
}

export function detailThreadSetCache( key: string, value, duration: number ) {
    detailThreadCache.set( key, value, duration )
}
\$\endgroup\$
3
  • \$\begingroup\$ There is more than one LRU cache module available for Node.js. Please be specific. Also, it would be a good idea to add some context about how you are using these functions. At first glance, I would suggest just getting rid of all four functions. \$\endgroup\$ Commented Oct 24, 2016 at 15:35
  • \$\begingroup\$ I added link to the lru module in question. Would be very curious to see how you would replace the functions, while still allowing use of several different caches. \$\endgroup\$ Commented Oct 24, 2016 at 15:49
  • 2
    \$\begingroup\$ Like I said, just get rid of them, period. If you want to see specific examples of how to do that, then show your code. Otherwise, you're just asking for code to be written rather than asking for a code review. \$\endgroup\$ Commented Oct 24, 2016 at 15:51

1 Answer 1

1
\$\begingroup\$

As mentioned in the comments, remove the functions. Don't even go into OOP. As to why, they're simply unnecessary wrappers to your cache objects which you can just access directly. detailThreadCache.set is just as verbose as detailThreadSetCache.

Improving code doesn't necessarily mean reuse or encapsulation or a paradigm shift. Often times, the best code... is the one that's never written.

\$\endgroup\$
3
  • \$\begingroup\$ Ok I see what they meant when they recommended to remove the functions.But what if I decide in the future to replace leru memory cache with redis ? With the wrapper I would only change the cache file and not the places where the functions are called. Wouldn't it be worth it ? \$\endgroup\$ Commented Oct 24, 2016 at 20:34
  • 1
    \$\begingroup\$ @RobertBrax But that remains to be seen, a "what if". Analysis paralysis is real. Focus on your app, worry when you get there. Regardless of the presence an abstraction layer, a switch in dependency will almost always involve a significant rewrite. It's never a "swap out and it will just work". \$\endgroup\$
    – Joseph
    Commented Oct 24, 2016 at 23:11
  • \$\begingroup\$ Indeed, I removed the file, and made direct call to the cache module. Result: I eliminated an entire file, and simplified the problem of the different caches, since each call is on a different file with its own variable storage. Also you are so right, it's never a "swap and it will work" anyway. \$\endgroup\$ Commented Oct 25, 2016 at 11:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.