2

I have a function which modifies a variable as per response from database.

function f () {
     let x = { }; 

     redis.get ("key1").then ( value => x.prop1 = value);

     redis.get ("key2").then ( value => x.prop2 = value);

     return x;
}

This is my code roughly. It is actually in a for loop, using node-redis.

I have multiple calls to Redis. I add the data from Redis to a new object and return the object.

PROBLEM: function f () is returning an empty object every time I call it.

How do I wait for all Redis promises to resolve before returning the object?

vixrant
  • 178
  • 3
  • 12

1 Answers1

2

In your code, you are not waiting for redis values to resolve before you are returning x. You should enclose it in async-await for that.

Also, note that invoking f() will not return the value directly, it will return a promise.

async function f () { // note async
     let x = { }; 

     await redis.get ("key1").then ( value => x.prop1 = value); // note await

     await redis.get ("key2").then ( value => x.prop2 = value);

     return x;
}

You can then use it like f().then(console.log)

Example:

//var Promise = require('bluebird')
console.log("started");
var f = async function() {
    return { 
      prop1: await Promise.delay(3000).then(_ => 3),
      prop2: await Promise.delay(2000).then(_ => 4)
    }
}

f().then(console.log)
<script src="https://cdn.jsdelivr.net/bluebird/3.5.0/bluebird.js"></script>

which can also be simplified to:

async function f () {
     let x = { }; 

     return { prop1: await redis.get('key1'), prop2: await redis.get('key2') }  
}

Edit: On thinking about this more, I think you should just rather move away from async-await for this use-case, and instead use Promise.all. Reason - those two redis get operations can happen concurrently and shouldn't wait for it's turn. Using await will stall the invocation of second redis call. Try running the example snippet above, and you will notice how the result comes in total 5s (3 + 2), while it easily come in ~3s.

Promise.all accepts a bunch of promises and waits for all of them to resolve before invoking it's resolution.

console.log("started");
var f = function() {
    var x = {}
    var promises = [
        Promise.delay(3000).then(_ => { x.prop1 = 3 }),
        Promise.delay(2000).then(_ => { x.prop2 = 4 })
    ]
    // wait for all of above to resolve and then resolve with `x`
    return Promise.all(promises).then(_ => x)
}

f().then(console.log)
<script src="https://cdn.jsdelivr.net/bluebird/3.5.0/bluebird.js"></script>

^ Try running this snippet now and note the difference in time.

kiddorails
  • 12,961
  • 2
  • 32
  • 41
  • 1
    @JonasLochmann dude. relax. I just wanted to post answer for my own, not to make you happy. If I wanted to actually copy your answer, I would have copied it without `x`. If I wanted to copy your answer, I wouldn't have cared to experiment on my own and add some example which actually makes sense. – kiddorails Jul 01 '18 at 06:19
  • Guys please don't fight... Thank you both for your efforts. – vixrant Jul 01 '18 at 06:24
  • 1
    @Vikirnt No. I'm not even taking this as fight. by the way, I just edited my answer to add a **better** alternative which should actually make sense with your use-case. May be helpful for your knowledge and journey with promises :) – kiddorails Jul 01 '18 at 06:28
  • 2
    @jonas lochmann you really think that you can declare ownership for such a simple snippet?! This answer is way better as it includes a proper explanation. – Jonas Wilms Jul 01 '18 at 08:33
  • Promise.all is fantastic. Thanks! – vixrant Jul 01 '18 at 10:02