0

I want to read data from Firebase Real-Time Database and Firebase Storage. Reading RTDB happens first. This is because the name of the files that are stored in Firebase Storage are the keys of the data in the RDTB. I have the following code:

let newReadRef = firebase.database().ref('someRef').limitToLast(3).once('value').then(snapshot => {
  let promiseArray = [];
  snapshot.forEach(e => {
    promiseArray.push(new Promise((resolve, reject) => {
      //Do all the things I need to do before trying to get data from my Firebase Storage

      let filePathRef = firebase.storage().ref(e.val().key);
      filePathRef.getDownloadURL().then(url => {
        //Do something with the url
        resolve('Promise resolved');
      }).catch(e => {
        console.log(e);
        reject('Rejected');
      });
    }));
  });

  Promise.all(promiseArray).then(someFunction);
});

The idea is:

  1. Retrieve some data from my RTDB, its key will be used later to retrieve data in my Firebase Storage
  2. Create a new Promise for each data read in my RTDB
  3. For each Promise created, read data from Storage using key obtained from RTDB data. Resolve the Promise when data is read successfully. Otherwise, reject.
  4. After all Promises have been created, when all Promise resolves, do something using someFunction that will alter the data created

It seems like my code is a little too nested. Is there a better way to achieve the above?

Richard
  • 7,037
  • 2
  • 23
  • 76
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! Also you should [unnest](https://stackoverflow.com/a/22000931/1048572) the final `.then(someFunction)`. – Bergi May 13 '19 at 06:19
  • @Bergi By unnesting the final `.then(someFunction)`, do you mean placing the `Promise.all(promiseArray).then(someFunction);` inside a new `then` in my `newReadRef` Promise-returning function? – Richard May 13 '19 at 06:35
  • @Bergi I don't think I'm using the antipattern you linked? – Richard May 13 '19 at 06:35
  • I mean you should `return` the `Promise.all(promiseArray)` and then chain `.then(someFunction)` on the outer promise. And yes, you are using the antipattern - Jaromanda's answer shows how to avoid `new Promise` here. – Bergi May 13 '19 at 06:38
  • @Bergi I see. Upon looking again, yes it does have that antipattern on the link you provided (the right code). Thank you for the response :-D – Richard May 14 '19 at 02:17

1 Answers1

2

No, the code is about as flat as you can get it, though, since the last thing you're doing returns a Promise, you won't need to create a Promise

snapshot.forEach(e => {
    //Do all the things I need to do before trying to get data from my Firebase Storage
    let filePathRef = firebase.storage().ref(e.val().key);
    promiseArray.push(
        filePathRef.getDownloadURL()
        .then(url => {
            //Do something with the url
            return 'Promise resolved';
        }).catch(e => {
            console.log(e);
            throw 'Rejected';
        });
    );
});

I was going to suggest using .map instead of .forEach, but snapshot isn't a Javascript array if I recall (it has a forEach, but no map method)

Jaromanda X
  • 53,868
  • 5
  • 73
  • 87