Actionscript 3 while loop with indexOf

Question

Hi,

Is there any reason why this loop is getting stuck? I can't quite get my head around it:

var i:Number = -1;
do
{
i = Math.round(Math.random() * _totalQuestions);
}
while(_usedQuestions.indexOf(i));

Where _usedQuestions is an array of numbers. This array starts empty.

Thanks!

Edit: I want the loop to end if i is NOT found in the array.. this way I know the question I have selected has not previously been asked.

Answer 1

Try this.

    var i:Number = -1;
    var found:Boolean;
    do
    {
        i = Math.round(Math.random() * _totalQuestions);

        found = false;
        for (var j:int = 0; j < _usedQuestions.length; j++)
        {
            // assume that _usedQuestions is an array which holds the question
            // number that has been selected before
            if (_usedQuestions[j] == i)
            {
                found = true;
                // get out of for loop if found, we'll need to get another
                // random number
                break;
            }
        }
    }
    while (found);  

I'm not really proficient with AS3 since just started learning it so someone might have a more efficient way to do the above.

Answer 2

indexOf will return -1 if there is nothing found so use that value, also for avoiding an infinite loop check that _useQuestions does not contains all your totalQuestions :

Edit : a more complete version to illustrate:

var checked:int = 0;
var seen:Dictionary = new Dictionary();
while (checked < _totalQuestions) {
    var i:int = int(Math.random() * _totalQuestions);
    if (_usedQuestions.indexOf(i) < 0) {
        break;
    } else if (seen[i] === undefined) {
        seen[i] = i;
        checked++;
    }
}
if (checked < _totalQuestions) {
    //ok you have found a non used questions
} else {
    // all questions have been used
}

Answer 3

There are really two answers here.

1. The loop never exits because Array.indexOf() returns -1 if the argument is not found, and -1 is true in a boolean context. The only way your loop will ever exit is if i happens to be equal to _usedQuestions[0].

2. It may not be obvious, but even if you fix the above problem your loop will still fail to exit once all the questions have been used... and that's your real problem - you're using a confusing algorithm to do something simple.

It would make a lot more sense to simply keep two arrays - one of unseen questions and one of seen questions. Every time you choose a new question, simply remove it from unseen and add it to seen. Like this:

if (unseen.length > 0) {
    var i:int = Math.floor( Math.random() * unseen.length );
    seen.push( unseen[i] );
    unseen.splice(i, 1);
} else // all questions seen...

Remember: writing programs that work correctly is merely the minimum requirement of programming. A good programmer writes programs that are easily understood by people as well! And a big part of that is making sure that simple things are done in simple ways. (Unless performance is an inescapable factor - but in this case, I absolutely promise that it won't!)