Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I wrote a short program for my exercise routine. It takes a JSON array of exercise names and duration (seconds) and puts it on the screen. The JSON looks like this:

var exerciseArray = [
    {
        "desc":       "Pushup/Pullup",
        "countdown":  30
    }, {
        "desc":       "Squat punch",
        "countdown":  45
    }, {
        "desc":       "Jumping jacks",
        "countdown":  60
    }
];

The function works, but it seems to be to be kind of a kludge. My problem was that I couldn't figure out how work with setInterval to iterate through the list of exercises (exerciseArray). What I did in the end is just decrement the countdown value, and when it's zero, move to the next item in exerciseArray (i++;). When I hit the end of the array — as measured by i and exerciseArray.length — I just run clearInterval and I'm done. Code below (I removed some of the extraneous stuff that just updates the HTML with the number of seconds, you can see the full thing on github):

function exerciseTimer(exerciseArray) {
    var i = 0;
    var exerciseObject = exerciseArray[i];
    // do stuff
    var tt = setInterval(function() {
        var exerciseObject = exerciseArray[i];
        // write html to the page
        exerciseObject.countdown = exerciseObject.countdown - 1;
        if (exerciseObject.countdown <= 0) { 
            if(i < (exerciseArray.length - 1)) {
                i++;
            } else {
                clearInterval(tt);
            }
        }
    }, 100);
}

My question is, what is the appropriate way to iterate while using setInterval? Is there a cleaner way? This feels very ugly to me.

share|improve this question
    
Is your intent to have the code write some HTML, wait until that exercise timeout, and then write some different HTML and then wait for the next timeout, or are you doing updates or executing other code during the period of each exercise? –  Edward May 5 at 14:07
    
@Edward - There are updates during the exercise; namely, the timer counts down. –  eykanal May 5 at 19:00

2 Answers 2

up vote 4 down vote accepted

If you only need the entries once, you could simply keep using shift until there are no more entries:

var exercises = [
    {
        "desc":       "Pushup/Pullup",
        "countdown":  30
    }, {
        "desc":       "Squat punch",
        "countdown":  45
    }, {
        "desc":       "Jumping jacks",
        "countdown":  60
    }
];

function exerciseTimer(exercises) {
    // do stuff
    var exercise = exercises.shift(),
        tt = setInterval(function() {
            // write html to the page
            exercise.countdown = exercise.countdown - 1;
            //Does it work ?
            console.log( exercise );
            if (exercise.countdown <= 0) { 
                exercise = exercises.shift(); 
                if(!exercise) {
                    clearInterval(tt);
                }
            }
        }, 100);
    };

exerciseTimer( exercises );

I would also suggest to not make the type of the variable part of the variable name. exercise and exercises are much easier to read than exerciseArray and exerciseObject.

share|improve this answer

Assuming that what you want is to iterate through each exercise, display something static and wait for the timeout to expire before moving on to the next, I might do it like this:

<h1 id="exercisename">Get Ready</h1>
<h2 id="seconds">0</h2>
<p>Click the button to start the exercises.</p>
<button onclick="nextExercise()">Begin</button>  
<script>
var exercises = [
    {
        "desc":       "Pushup/Pullup",
        "countdown":  3
    }, {
        "desc":       "Squat punch",
        "countdown":  4
    }, {
        "desc":       "Jumping jacks",
        "countdown":  6
    }
];
var excount = 0;
var timeleft = 0;
var secondsTimer;
function setSeconds(s)
{
    var sec = document.getElementById('seconds');
    sec.textContent = timeleft;
}
function tick()
{
    setSeconds(timeleft--);
    if (timeleft <= 0){
        clearInterval(secondsTimer);
    }
}
function nextExercise()
{
    var ex = exercises[excount++];
    var name = document.getElementById('exercisename');
    if (typeof ex == 'undefined') {
        name.textContent = "Done";
        excount = 0;
    } else {
        name.textContent = ex.desc+" for "+ex.countdown+" seconds";
        timeleft = ex.countdown;
        setSeconds(timeleft);
        secondsTimer = setInterval(tick, 1000);
        setTimeout(nextExercise,ex.countdown*1000);
    }
}
</script>

Obviously for a real workout, you'd probably want to increase the times a bit. :)

Update:

Based on a comment, I've also added a countdown timer. Note that the countdown is handled as using setInterval while the exercise timers use a separate and independent setTimeout.

share|improve this answer
    
The way this works now, I have it counting down for me. I use the setIterator to change the displayed second as time progresses. This is a minor decrease in functionality. –  eykanal May 5 at 15:52
    
I've added an additional timer to handle the countdown timer. –  Edward May 5 at 16:20

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.