2

I am using it many places in my code and it works. but at one place it didn't give any error, also did not give me the desired result. and when I show my code to the support forum they suggest that "You are using the JS object/class “Promise” incorrectly."

Can Anyone guide me on what's wrong with my code here is my code sample:

let charity = {};
      await Promise.all(
        charity = charityData.map(function( data ) {
              let address = data.zipCode
              let url = "https://maps.googleapis.com/maps/api/geocode/json?&address="+`'${address}'`+"&key=***Google geocoding Key***"; //client's Key
              let urlResponse = Backendless.Request.get(url)    
               // let latitude = urlResponse.results[0].geometry.location.lat;
               // let longitude = urlResponse.results[0].geometry.location.lng;
              //let updateCharitiesData = {'objectId': data.objectId, 'latitude':latitude, 'longitude':longitude};
              return urlResponse;
        })
      );
    return charity;
shahrukh khan
  • 21
  • 3
  • 1
    As a general rule, I think it's a bad idea to use a singular noun (charity) to represent multiple things (in this case, an array of promises). It expresses the wrong thing. – jarmod Dec 01 '21 at 17:30
  • 2
    Your question would be better if you explained the result you expected and the result you received. You will get answers that fix it either way but with that context someone will be better positioned to explain why it didn't work for you and why their suggestion is an improvement in this specific situation. – Rodger Dec 01 '21 at 17:48
  • 1
    "*it did not give me the desired result*" - what is the desired result?! – Bergi Dec 02 '21 at 01:50
  • 2
    `let charity = {}; charity = charityData.map(…); return charity` is really weird. What did you intend to achieve by that? – Bergi Dec 02 '21 at 01:53

3 Answers3

2

Almost. Assuming Backendless.Request.[method] returns a promise it would be more correct to do something along the lines of:

async function getCharityData() {
    const charity = await Promise.all(charityData.map( async function(data) {
       const address = data.zipCode;
       const url =
        `https://maps.googleapis.com/maps/api/geocode/json?&address=${address}&key=***Google geocoding Key***`; //client's Key
       const urlResponse = await Backendless.Request.get(url);
       return urlResponse;
   }));

   return charity
}

Promise.all requires an array as its argument to work correctly; passing an Array.map here and assigning the returned value to charity both ensures your Promise.all runs as expected and the returned array is an array of resolved promises.

Joshua Burleson
  • 21
  • 2
  • 1
    This is an answer to the question but it would be much improved if you explained why to do it this way and/or why the OP's code works in some areas and not others. – Rodger Dec 01 '21 at 17:47
  • Why did you change the `map` callback to be an `async` function? – Bergi Dec 02 '21 at 01:51
  • @Bergi because it seems that urlResponse should be awaiting the get request. – Joshua Burleson Dec 02 '21 at 14:44
  • 1
    Thanks @Rodger, I've updated the answer to explain. – Joshua Burleson Dec 02 '21 at 14:46
  • @JoshuaBurleson Yeah, but you could have renamed it to `const urlResponsePromise` and just `return`ed that :-) Or omitted the temporary variable alltogether. – Bergi Dec 02 '21 at 19:40
  • @Bergi you certainly could; but I would prefer to make it clear as possible for more junior engineers (or that are just newer to the JS/TS ecosystem) what's going on. There's really no drawback (in this case, since we don't really need to concern ourselves with the memory allocation here), but you're right, it isn't needed per se. – Joshua Burleson Dec 03 '21 at 19:09
1

I would do it like this:

function getCharityData() {
    // `charity` is an array of Promises that will each resolve to 
    // a response.
    const charity = charityData.map((data) => {
        let address = data.zipCode;
        let url = 'https://maps.googleapis.com/maps/api/geocode'
        let urlResponse = Backendless.Request.get(url);
        return urlResponse;
    });
    return Promise.all(charity);
}
try {
    const charityData = await getCharityData();
} catch (e) {
    console.error(e);
}

This way, charityData will be an array of fetched responses.

In your code, the result of Promise.all() is never assigned to charity before it's returned, and that is the value you want.

Dmitry Minkovsky
  • 36,185
  • 26
  • 116
  • 160
0

If you have access to Async/Await I'd simply do the following:

function getCharityData(charityData) {
   let results = [];

    for (let i = 0; i < charityData.length; i++) {
       let url = `https://maps.googleapis.com/maps/api/geocode/json?&address=${charityData[i].zipCode}&key=***Google geocoding Key***`;

        try {
            let result = await Backendless.Request.get(url);

            results.push(result);
        } catch (err) {
            console.log("Oh dear!");
        }
    }

    return results;
}

For your use case, there's no need to use any Promise libraries when you have Async/Await, good old fashioned for loops and await (I'd personally prefer to do this sort of call sequentially instead of in parallel like Promise.all implores when I'm querying external APIs. This also ensures we don't fail fast like Promise.all does.).

MattGarnett
  • 545
  • 3
  • 20
  • 2
    The issue with this is that because it doesn't use `Promise.all`, the requests are made sequentially instead of in parallel, which is maybe what OP is trying to do. – Dmitry Minkovsky Dec 01 '21 at 17:18
  • 1
    `Promise.all` is not a "promise library" – Bergi Dec 02 '21 at 01:51
  • @Bergi "There's no need to use any Promise libraries" refers to libraries such as q or Bluebird. That was my whole point - you don't need to use a promise library. If you want to suggest an edit for my wording go ahead. – MattGarnett Jan 17 '23 at 02:11