.. instead do it a smarter way, by creating an abstraction to rely upon, using the built-in Promise object in JavaScript..
A movie once began... A LONG TIME AGO IN A GALAXY FAR FAR AWAY....
When your code depends on data that is 'far away', in other words expensive to obtain, it's common to utilize a cache.. However it is notoriously common to find code that depends on a cache to a certain boilerplate ugly block of code, whose looks and behavior are actually more insidious than at first glance. I'll explain through examples why this is a problem and how to write it cleaner.
Imagine you're in the future - where the only fuel your spaceship can use is Unobtainium.
You get this fuel, Unobtainium, by calling a function, which takes some time to return. Now the return value from this function will be a delayed value, so we implement it as a Promise slightly delayed, for a randomly determined amount, which is then turned into an object like the following:
// A function promising unobtanium: an object with keys 'type' 'units' and 'grade'
const getUnobtanium = (delay = 0.5) => {
return simulateDelay(delay)
.then(() => { return 3.14 + Math.round(Math.random()*2-1) /* simulated scarcity */})
.then(amount => ({ type: 'Unobtanium', grade: 'A++', units: amount }))
}
function simulateDelay(seconds) {
return new Promise(resolve => setTimeout(resolve, seconds*1000))
}
// Well use this later!
function turnOnMusic() {
console.log('More than a fee-linnnnnnnnnng')
}
await getUnobtanium(Math.random())
As you can see, this allows us to get a Promise for some grade A++ Unobtainium, at some random time in the future..
Now, because of this delay, it makes sense 1) to cache the result and 2) to have a part of the code that gets the possibly cached value.
However the most common naïve implementation has some serious flaws which a junior developer probably would not spot, but I'll lay it out for you below:
// lets use this as a standin for memcached or redis..
const cache = new Map
function startShipNaïvely() {
let fuel
if (cache.get('fuel')) {
fuel = cache.get('fuel')
} else {
getUnobtanium().then(fuel => {
/*
start the ship with the fuel
*/
cache.set('fuel', fuel) //and set it in the cache
})
}
}
Reviewing this code - a few questions come to mind.
- What happens if an exception occurs when starting the ship - does the fuel ever get cached?
- Is there a promise for the result of starting the ship? Example: startShip().then(turnOnMusic)
- Did the author of this code look for whether there were more concise ways of doing this in the codebase?
- Is every new method that touches the cache supposed to add all this boilerplate too?
'<img src="http://memedad.com/memes/172636.gif"/>'
Yeah, in the end it's a damning analysis. We can't take this code, and will have to decline the PR. The explanation will read briefly: "Needs fuller error-handling, and will bloat codebase if accepted." Surprised? Not seeing it? Well, sit down with me, and we'll make it right.
Let's look instead at what a cleaned up version will look like:
// It's a matter of taste whether you take a promise for fuel, or the actual fuel as an argument
function startShipSmartly(promisedFuel) {
return promisedFuel.then(fuel => {
/* start ship */
console.log('starting ship, vrooom...')
})
}
await startShipSmartly(Promise.resolve('simulated fuel'))
This architecture cleans up a number of things! From a readability perspective:
- We take the thing we need - the fuel - as an argument - this calls out the intent of the code
But our big other benefits are:
- 1) We have Zero logic about the cache - we have successfully DRYd up our code!
- 2) An exception in starting the ship will never fail to cache the fuel first!!
- 3) Copying and pasting this pattern around won't produce nearly the bloat of the other style
- 4) We now return a promise for the ship having started: startShipSmartly(fuel).then(turnOnMusic) now works!
You probably didn't catch point #2 yourself earlier, but its exactly why letting 'high level' code in your app knowing about operational details of caching is a Very Bad Idea.
But regardless - now we're in business. And I love that now you can simply do the following, and get a predictable, error-safe chain of functionality despite the async nature of getting the hardest element to obtain in the universe..
await startShipSmartly(getPromisedFuel()).then(turnOnMusic)
Congrats! You have an automatically-caching fuel system. And you are flying that hunk of space metal to the rockin' tunes of Journey without cluttering your codebase! And I knoww your next PR will follow a cleaner style ;)
As an appendix, I'm sure you want to see how you can get such clean syntax, and at the end I'll include one way - though there are many variants on this theme..
So there you have it - a few lines of code that can really shine when written one way, or be a real drag on your codebase if done another way. If you like this little code review lesson, please share it and ask me what you'd want to do next! You see - cleaning up code and being a hero on your team can be super-easy...
Or at least easier than obtaining Unobtanium!
Please clap like and share!
Dean
/* Sample implementaion of hiding caching logic from consumers (hard coded for one-key) */
// closes over our cache
function getPromisedFuel () {
return new Promise((resolve, reject) => {
console.log("getting fuel...")
if (cache.get('fuel')) {
return resolve(cache.get('fuel'))
}
// omitting the try/catch around this for demo purposes
return getUnobtanium(.1).then(fuel => {
console.log("got fuel, storing for later use")
cache.set('fuel', fuel)
return resolve(fuel)
}).catch(ex => reject(ex))
})
}
// To un-hardcode, you'd probably want the above abstracted a bit, with a function
// that can cache-wrap any function at any given key. The implementation of that
// will be left as an exercise to the reader :)
// const dataSources = {
// fuel: cacheWrap(getUnobtanium, 'fuel')
// }
await "Thanks for reading!"
1 comment
- posted 6 years ago byIn your last sample, it would be better, to cache the promise itself instead of the result. In your implementation, if you call `getPromisedFuel` twice asynchrously in a short time, the long function `getUnobtanium` will be called twice. This is how I would do it: https://runkit.com/nch3v/cache-promise-instead-of-value
sign in to comment