PHP recursive function is not working right

Question

I am running a function, and part of it requires to pick a random number. However, it cannot be in an array of already selected numbers. So if the random number chosen is in the array, it needs to select a new number. I seem to be really struggling with something that seems so easy.

function choice() {
    global $countries;
    global $count;
    global $answers;
    global $choice;

    $i = rand(0, $count - 1);
    if(in_array($i, $answers))
        choice();
    else {
        $answers[] = $i;
        $choice = $countries[$i]['capital_city'];
        return $choice;
    }

So the current logic here, is it selects a random number, checks if it’s in the array. If it isn’t then it sets the variable and returns it. If it is, then it generates a new number by restarting the function. When it does find one in the function, it returns an empty result, rather than going back through the function. What am I doing so wrong?

We can't someone who doesn't help us first by telling us what the precise error and location is — Andrew Li, Jun 28 '16 at 23:23
have a look at this question, seems very similar: http://stackoverflow.com/a/17110791/1823684 — Terry Kernan, Jun 28 '16 at 23:25
You know functions can take arguments, right? Use of globals is generally a bad thing and should only be used when that is your last possible pragmatic choice. — Mulan, Jun 28 '16 at 23:47

score 2 · Answer 1 · edited May 27 '22 at 13:19

2

If you don’t need to use recursion, you could just use a while loop:

function choice() {
    global $countries;
    global $count;
    global $answers;
    global $choice;

    $i = rand(0, $count - 1);

    while (in_array($i, $answers)) {
        $i = rand(0, $count - 1);
    }

    $answers[] = $i;
    $choice = $countries[$i]['capital_city'];
    return $choice;
}

Or use this do while in place of the while;

do {
    $i = rand(0, $count-1);
} while (in_array($i, $answers));

edited May 27 '22 at 13:19

Peter Mortensen

30,738
21
105
131

answered Jun 28 '16 at 23:32

atoms

2,993
2
22
43

1

count value didnt need to be altered. – Source Jun 28 '16 at 23:47
1

I think a do while loop would maybe look a bit better here :) – Rizier123 Jun 28 '16 at 23:51

score 2 · Accepted Answer · edited Jun 28 '16 at 23:45

2

Add a return statement before the recursive call.

function choice() {
    global $countries;
    global $count;
    global $answers;
    global $choice;

    $i = rand(0,$count-1);

    if(in_array($i,$answers)) return choice();
    else {
        $answers[] = $i;
        $choice = $countries[$i]['capital_city'];
        return $choice;
    }

edited Jun 28 '16 at 23:45

Rizier123

58,877
16
101
156

answered Jun 28 '16 at 23:35

etherealite

78
6

That seems to work, I knew i was missing something simple like that, but I couldnt work it out. – Source Jun 28 '16 at 23:42
However, what is better, this way, or using a while loop? – Source Jun 28 '16 at 23:42
I believe the while loop will have less overhead. Would also use references in place of global – atoms Jun 28 '16 at 23:43
3

@Source the "correct" answer is meaningless if this is the code you end up with. – Mulan Jun 28 '16 at 23:48
@naomik No regrets, #globals for life. – etherealite Jun 29 '16 at 00:07

PHP recursive function is not working right

2 Answers2