Promise design pattern

Question

I've created a SoundManager to deal with sounds in my application. In my app, I instantiate a SoundManager and then use it to load audio files, and return a promise once they're loaded.

I've omitted some of the code as i don't think is important in this context - I know they all load properly, for instance, the then() function in my app, which is passed the array of Promises from loadSounds() receives an object with all the sounds (not promises).

However, if from my App (at a point where the sounds are all loaded), I call playSound(), the this.sounds variable will be an array of Promises.

Why does that happen? Once the promise is fulfilled, shouldn't the variable this.sounds in SoundManager update?

Also that doesn't look like a good pattern, so any ideas on how to implement this?

import Promise from 'bluebird';
import {Howl, Howler} from 'howler';

export default class SoundManager {
  constructor(){
    this.sounds;
  }

  loadSounds(soundsArray){
    this.sounds = soundsArray.map((data) => {
      return new Promise((resolve, reject) => {        
        // ...
      })
    })
    return Promise.all(this.sounds)
  }

  playSound(id){
    console.log('this.sounds: ', this.sounds); //this is an array of promises, when I expect an array of sound objects
    let sound = this.sounds.filter((sound) => sound.id === id);
    if (!sound.length) {
      console.warn(`could not find sound with id "${id}"`)
      return false;
    }
    sound[0].play()
  }
}

Because... you set that property's value to an array of promises. Where's the confusion? It wouldn't make sense for that property to instead contain an array of values, those values didn't exist at the time. — Kevin B, Mar 01 '18 at 20:55
yes, that makes a lot of sense, I understand. I actually don't need to return nothing to the App (which calls SoundManager.loadSounds()), but I'd like it to load all files, store them in a variable and then return to my app, so it continues doing its stuff — zok, Mar 01 '18 at 21:05
it is a little silly. You can't set the value of this.sounds to an array of things that don't exist yet. That's why it is instead an array of promises. You could re-define it as an array of sounds later, after the promise has resolved, but you'd then have to make sure you don't access it until later too. — Kevin B, Mar 01 '18 at 21:09
A little verbose, but you should have two arrays in use here, an array of promises and then an array of sounds, that gets populated when said promises are fulfilled — andy mccullough, Mar 01 '18 at 21:13

score 2 · Accepted Answer · answered Mar 01 '18 at 21:13

this.sounds contains an array of promises because that's exactly what you assign to it here:

  loadSounds(soundsArray){
    this.sounds = soundsArray.map((data) => {
      return new Promise((resolve, reject) => {        
        // ...
      })
    })
    return Promise.all(this.sounds)
  }

The results of your soundsArray.map() is an array of promises.

If you wanted it to contain the eventual values, you could do that like this:

  loadSounds(soundsArray){
    let sounds = soundsArray.map((data) => {
      return new Promise((resolve, reject) => {        
        // ...
      })
    })
    return Promise.all(sounds).then(data => {
        this.sounds = data;
        return data;
    });
  }

Though doing this seems a bit problematic because other than using the promise that loadSounds() returns, there's no other way to know when this.sounds contains the data you want. So, if the only way to know when the data is there is the promise that loadSounds() returns, then it's not clear how other code would be able to reliably use this.sounds.

You could store the promise from Promise.all() in this.sounds and then any code that wanted the sounds could use this.sounds.then() to get it. We'd have to see more of what you're trying to do with this.sounds and when you're trying to do it to know what the best design would be.

score 1 · Answer 2 · answered Mar 01 '18 at 21:21

Once the promise is fulfilled, shouldn't the variable this.sounds in SoundManager update?

No, that's not how promises work. They stay promise objects, they do not "become" the value they were fulfilled with.

This doesn't look like a good pattern

Yes, don't load the sounds in the manager. The manager might or might not be usable during its lifespan - that's not particularly helpful. Better instantiate it only after loading the sounds - see this also here.

export default class SoundManager {
  constructor(sounds) {
    this.sounds = sounds;
  }

  static loadAndCreate(soundsArray) {
    return Promise.all(soundsArray.map((data) => {
      return new Promise((resolve, reject) => {        
        …
      })
    })).then(sounds => new this(sounds));
  }

  playSound(id){
    console.log('this.sounds: ', this.sounds); //this is always an array of sound objects
    let sound = this.sounds.find(sound => sound.id === id);
    if (!sound) {
      console.warn(`could not find sound with id "${id}"`)
      return false;
    }
    sound.play()
  }
}

Promise design pattern

2 Answers2