0

This code randomly plays Audio elements. Running setup twice allows you to do this with two different arrays simultaneously, which is what I want. The problem is that #stop only stops one of the arrays playing. This actually also happens if you only call setup on one array, but click #start more than once (which I also don't want). I figure this has to do with 'intervalReturn', as it would only be specified to one setInterval.

How should I write this so that multiple invocations of setup creates distinct setIntervals which can be started only once?

Alternately, if I should approach this from a totally different angle, what would be better?

EDIT: This was fixed per the suggestions below. But I'm wondering, what is going on "under the hood" with setInterval here? Why does this behavior happen at all? (Specifically: #stop stops one but not all audio elements.)

var CMajor3 = new Array ("C3","D3","E3","F3","G3","A3","B3","C4a");
var CMajor4 = new Array ("C4b","D4","E4","F4","G4","A4","B4","C5");

var intervalReturn = null;  

function pickAndPlay(pitchSet){
    fyshuffle (pitchSet);  // the Fischer-Yates shuffle function
    var tone = document.getElementById(pitchSet[0]);
    tone.currentTime = 0;
    tone.play();    
};

function setup(time, pitchSet){
    $("#start").click(function() {   
        console.log("startClicked"); 
        intervalReturn = window.setInterval(pickAndPlay, time, pitchSet)
    }); 
    $("#stop").click(function() {   
        console.log("stopClicked");  
        window.clearInterval(intervalReturn)
    }); 
};  

$(document).ready(function() {  
    setup(2000, CMajor3);
    setup(2000, CMajor4);
}); 
9
  • 2
    the problem is the single intervalReturn variable can't hold two different values simultaneously! - try moving var intervalReturn = null to be the first line in function setup Commented Jun 16, 2017 at 2:44
  • see the edit to the comment? Commented Jun 16, 2017 at 2:47
  • 1
    "This...also happens if you...click #start more than once." - Is that something that you want to be able to do? Or should the user have to stop the previous one before clicking start again? Commented Jun 16, 2017 at 2:53
  • 1
    You could do if (intervalReturn !== null) as long as you explicitly set it back to null in the #stop function. Commented Jun 16, 2017 at 2:56
  • 1
    Oh, yeah, sorry, you'd want the opposite, if (intervalReturn === null) then call setInterval() in the #start function, and reset it to null in the #stop. In other words, "if not already running then start..." Commented Jun 16, 2017 at 3:07

1 Answer 1

1

But I'm wondering, what is going on "under the hood" with setInterval here?

Each time you call setup(), that creates additional click handlers on the #start and #stop elements. When you actually click #start, or #stop, all of the applicable handlers are called (in the same order they were bound). This is why clicking #start causes both CMajor3 and CMajor4 notes to play. You get multiple concurrent but unrelated intervals running.

With intervalReturn defined as a global variable, you only ever have the interval ID that was returned from the most recent call to setInterval(), because each time the #start click handler runs it overwrites the previous one. That's why clicking #stop only ever stops one of the intervals and there is no way to stop the others.

Moving the var intervalReturn declaration inside the setup() function helps because the way closures work in JS is that the arguments and local variables of setup(), i.e., time, pitchSet and intervalReturn (after you've moved the declaration) are accessible in the two event handlers defined in the current call to setup(). Subsequent calls to setup() create new closures with their own separate copies of those variables. So then the #stop click handler uses the individual intervalReturn relevant to its own setup(). And since both #stop handlers run, both intervals get cleared.

But you still have the problem that clicking #start# more than once without clicking #stop creates additional intervals and then within any one setup() that individual intervalReturn gets overwritten with the latest, so again #stop has no way to refer back to the previous intervals. Which is why adding if (intervalReturn === null) in the #start handler helps to only start a new interval if there is not already one running. (And then you need to add intervalReturn = null in the #stop handler because just calling clearInterval(intervalReturn) doesn't change the value of the intervalReturn variable.)

Suggestion: Update your existing console.log() statements to the following:

console.log("startClicked", pitchSet, intervalReturn); 
// and
console.log("stopClicked", pitchSet, intervalReturn); 

And maybe more the startClicked one to just after calling setInterval() so that it logs the interval ID that was just returned, not the previous one. That way you can see the values of all of the relevant variables and see what is happening.

Sign up to request clarification or add additional context in comments.

1 Comment

Amazing! Thank you greatly for this thorough explanation. It makes it much more clear what I need to study up on to understand this project.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.