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;
 };

• Schism: It would help greatly if you indicated what this was, as well as stuff like the context for masterVolume and instanceVolume -- are these globals? Having the enclosing closure would be super useful for reviewing.
• Andrew: @Schism It's apart of a larger system, and it's enclosed inside of a media object. I'll try uploading a better view of it.

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?

• Andrew: liveSounds will note work with a name key because every time a new sound is played a new live sound is added, and if it has the same name, that previous sound will be lost.
• George Mauer: @Lemony-Andrew ok, so that was something that I wasn't clear on what you desired - in this case an object where the values are arrays {name: [sounds]} might still be a better fit. As for splice - fair enough, but I guarantee you in 90% of the code you see, copying is done with slice. Might as well do the conventional thing and save yourself a letter :)
• rolfl: @Lemony-Andrew - note I rolled back your code-edit. Please post a follow-on question instead (see the comment I added to your question for a link).
• George Mauer: @rolfl I think that edit was with the more info as we requested, I don't think there was enough time to incorporate my feedback
• Andrew: @GeorgeMauer Observing your code more I have to say that there is a lot that I don't see fit in my code. Especially the this.sound function. The prototype needed to be called because I was overwriting the play() function of the audio instance. I use this because it stills refers to the instance it's attached too. "this.onended = function(){ liveSounds.splice( liveSounds.indexOf(snd), 1 );};" was absolutely mandatory for my code to work and clean itself out of the live code. Sounds aren't reset, if anything they're being killed, and your liveSoundsExcept function reversed my code...