views:

544

answers:

10

Why is it this sometimes returns 2?

function pickServer(){
    $varr = rand(1,4);
    if($varr==2){
        pickServer();
    }
    return $varr;
}
+19  A: 

Because you're not stopping the function there. the fourth line should read:

return pickServer();
nickf
This is still recursive and potentially dangerous. See the comment-discussion on the original question.
Robert Cartaino
This is useful for answering the "why." But it results in a dreadful solution, as Robert points out.
Nosredna
+2  A: 

Because when the value is 2 you don't return pickserver. And the function continues to return $varr.

MrHus
A: 

It calls the function recursively to get another number that isn't 2, but does nothing with that result.

Try changing pickServer() to return pickServer().

Better still, write the function iteratively so that it just loops until the value returned isn't 2.

Kylotan
+2  A: 
function pickServer(){
    $varr = rand(1,4);
    if($varr==2){
        return pickServer(); //leave here
    }
    return $varr;
}
Kai
+10  A: 

What you probably want is

function pickServer(){
  $varr = rand(1,4);
  if($varr==2){
    $varr = pickServer();
  }
  return $varr;
}

-- but note that there's no guarantee that this doesn't go into a too long recursion. Maybe you should rather do something like this:

function pickServer(){
  $varr = rand(1,3);
  if($varr > 1){
    $varr = $varr + 1;
  }
  return $varr;
}
balpha
Works as well as the one by Nosredna, but had to think a sec.
OIS
Your second piece of code deals with the issue well. My concern with it is only that it's a bit tricky to decipher. In fact, I missed it when I was reading the answers the first time through precisely because it's non-obvious. I apologize for missing it. Had I noticed it the fist time, I would have credited you in my answer.
Nosredna
Update: I gave you props in my answer.
Nosredna
Can't remember enough PHP, but out of curiosity, could you do $varr+=$var>1; in PHP? (I know it's terrible-looking.)
Nosredna
@Nosredna: What's PHP?
balpha
I meant $varr+=$varr>1;
Nosredna
A: 

You forgot to return the value...

function pickServer(){
$varr = rand(1,4);
if($varr==2){
    return pickServer();
}
return $varr;
}
rikh
+12  A: 

Another way to do it is using do … while:

function pickServer() {
    do {
        $varr = rand(1,4);
    } while ($varr == 2);
    return $varr;
}
Gumbo
Arguably better since most languages don't optimize tail recursion and this is not any less clear than the recusive one.
Adam Luter
Finally someone with some sense to point out that using recursion as a goto is crazy. +1
JohnFx
I think for the problem at hand, both recursion and looping are crazy. :-)
Nosredna
The second best answer, but far behind Nosrednas.
OIS
+1 because code is concise and conveys intent well.
JeffH
+26  A: 

The answer to your question, as others have pointed out, is that your code falls through without returning. If 2 is returned by the call to rand() on both the first attempt and the second attempt (there's a 1/16 chance of this happening), you'll get 2 as a result.

But your approach to solving the problem could be better.

Both recursion and looping are barmy for this problem. This is a mapping problem, not a randomness problem. (It resembles some common randomness coding interview problems which can be handled most easily in a rejection loop, but it really isn't a problem of that class.)

You want one of three outcomes, not four. (1, 3, and 4.) That means you should be generating a range of three random numbers, not four. You could remap with an array or use an if. Both possibilities are shown below. Let me know if I have syntax wrong--my PHPfu is weak this morning.

/* array remapping */
function pickServer() {
    $remap = array(1, 3, 4);
    return $remap[rand(1,3)];
}

/* if remapping */
function pickServer() {
    $server = rand(1,3);
    if ($server==2) {
        $server=4;
    }
    return $server;
}

I didn't notice it before, but balpha anticipated my answer. He remapped with an if in his second example. Instead of remapping 2 to 4, he just added one to any answer above 1, which is an equivalent solution.

Nosredna
The syntax looks correct to me. Well done. *This* should have been the selected answer.
Robert Cartaino
A true random RNG could return 2 forever. At least this will not get stuck in a loop.
OIS
I think you'll find that a true RNG always returns 4: http://imgs.xkcd.com/comics/random_number.png
nickf
+1. The original function and accepted answer are both criminally inefficient. No matter how many "holes" are in your range, you can always generalize it as a mapping problem.
Aaronaught
+1  A: 

I would so something like this:

function pickServer()
{
$servers = array(1,3,4);
return $servers[rand(1,count($servers))]; 
}
John Isaacks
I thought rand() was called with either zero or two parameters in PHP.
Nosredna
rand returns integers. I think this is what you were going for... return $servers[rand(1,count($servers))];
Nosredna
Thanks Nosrenda, I updated my code.
John Isaacks
A: 

You can remove the recursion and remap a randomly selected 2. Simply decrease the range and map the beginning of the range (in this case 2) to 1.

function pickServer(){
    $varr = rand(2,4);
    if($varr==2){
        return 1;
    }
    return $varr;
}
pdavis