Sound Control System

Question

This is my sound system that allows for the following functionality:

• One master volume that controls every sound.
• A function to stop/pause every playing sound
• A function to control every instance of a sounds volume(@creation)
• A function to stop/pause every instance of the same sound
• Sounds that are playing are kept in a list to keep track of.
• retrieve a list of specified sounds playing
• Retrieve a list of every sound playing

How could I possibly improve the code, or the system itself? I could possibly allow the user to define layers to encapsulate sectors of sound and not just the same instance. I could probably make a user.

  // exponentially decrease sounds
  this.Sound.masterVolume = function(v){ masterVolume = Math.pow(v,2);};
  this.Sound.instanceVolume = function(name, v){ instanceVolume[name] = Math.pow(v,2);};
  this.Sound.masterStop = function(){
    liveSounds.forEach(function(snd){
      snd.pause();
      snd.onended();
      snd.currentTime = 0;
    });
  }
  this.Sound.masterPause = function(){
    liveSounds.forEach(function(snd){
      snd.pause();
    });
  };
  this.Sound.instanceStop = function( name ){
    var match = sounds[name].src;
    liveSounds.forEach(function(snd){
      if( snd.src !== match ) return;
      snd.pause();
      snd.onended();
      snd.currentTime = 0;
    });
  };
  this.Sound.instancePause = function( name ){
    var match = sounds[name].src;
    liveSounds.forEach(function(snd){
      if( snd.src !==  match ) return;
      snd.pause();
    });
  }
  this.Sound.instanceGetLive = function( name ){
    return liveSounds.filter(function(snd){
      return( snd.src === match );
    });
  }
  this.Sound.masterGetLive = function(){
    return liveSounds.splice();//only a copy. 
  }

  this.sound = function(name){
    var snd = new Audio( sounds[name].src ),
        that = this;
    snd.volume = instanceVolume[name] || 1;
    snd.play = function(){
      this.volume *= masterVolume;
      Audio.prototype.play.apply(this);
      liveSounds.push(this);
      this.onended = function(){ liveSounds.splice( liveSounds.indexOf(snd), 1 );};
    };
    return snd;
  };

Answer 1

You can get really far cleaning this up by just creating some private functions and bumping them to the bottom of the file so that they can be hoisted

  this.Sound.masterVolume = function(v){ masterVolume = Math.pow(v,2);};
  this.Sound.instanceVolume = function(name, v){ instanceVolume[name] = Math.pow(v,2);};
  this.Sound.masterStop = function(){ liveSounds.forEach(reset) }
  this.Sound.masterPause = function(){ liveSounds.forEach(pause) }
  this.Sound.instanceStop = function( name ){ liveSoundsExcept(name).forEach(reset) }
  this.Sound.instancePause = function( name ){ liveSoundsExcept(name).forEach(pause) }
  this.Sound.instanceGetLive = function( name ){
    //this seems to be an error - what is match?
    return liveSounds.filter(function(snd){
      return( snd.src === match );
    });
  }
  this.Sound.masterGetLive = function(){
    return liveSounds.slice(0);//slice generates a copy - I don't think splice does
  }

  this.sound = function(name){
    var snd = new Audio(getSound(name)); //reference to `this` removed because it wasn't being used
    snd.volume = instanceVolume[name] || 1;
    snd.play = function(){ 
      //why use the ambiguous `this` here? It can vary depending on how this is called - you already have an isntance of `snd`!
      snd.volume *= masterVolume;  
      //Audio.prototype.play.apply(snd); //Why? you can just use 
      snd.play();
      liveSounds.push(snd);
      // This can't be right - you don't need to do any cleanup?
      // this.onended = function(){ liveSounds.splice( liveSounds.indexOf(snd), 1 );};
    };
    return snd;
  };

  function getSound(name) { return sounds[name].src}
  function liveSoundsExcept(name) {
    var match = getSound(name);
    return liveSounds.filter(function(snd){ return snd.src !== match });
  }
  function pause(snd) { snd.pause(); }
  function reset(snd) {
    pause(snd);
    snd.onended();
    snd.currentTime = 0;
  }

I absolutely agree with @Schism that more context is needed for a really good review but I made a few other observations inline.

One that also jumps out at me is this whole relationship between liveSounds and how you're selecting by name. Without more context I can't say for sure, but it seems like maybe you can have liveSounds = {} with the name being the key and that would make everything much simpler.

You do need to think if sounds need to get disposed of or what happens if you invoke a method for something without a name, or if there's two things with the same name.

Also, as always, I'm going to go ahead and say the prodigious use of this here is probably entirely unnecessary and you can just use a factory method instead of a constructor.

Also, it's 2015, can't we just start using es6 already?