next topic »

[on:]

Hi, can someone please take a look at my program and help me figure out where my logical error is.  I'm pretty sure the problem is in my if..else construction.

This is a link to a blog post describing the program:
http://www.meanpc.com/2012/01/arduino-simulation-of-blind-draw.html

The total number of 'wins' for players 1-6 should be 32000, but I'm getting totals of over 45,000 on test runs. 

Code:

void setup() {
  // put your setup code here, to run once:
Serial.begin(9600);

}

void loop() {
randomSeed(analogRead(0));
int player1,player2,player3,player4,player5,player6,iterations,i;
iterations = 32000;

// Pick a random number for the current player.  If the current
// player picks '1', add 1 to their total.  Otherwise draw again
// for the next player, subtracting 1 from the number of picks to
// draw from.

for (i = 0;i<iterations;i++){
if (random(1,7)==1) {player1++;}
else if (random(1,6)==1) {player2++;}
else if (random(1,5)==1) {player3++;}
else if (random(1,4)==1) {player4++;}
else if (random(1,3)==1) {player5++;}
else player6++;
}
// If the draw is a multiple of 1,000 - print the current results to the Serial monitor.

//if ((i%1000)==0) {
Serial.print("# ");
Serial.print(i);
Serial.print(" Plr 1: ");
Serial.print(player1);
Serial.print(" Plr 2: ");
Serial.print(player2);
Serial.print(" Plr 3: ");
Serial.print(player3);
Serial.print(" Plr 4: ");
Serial.print(player4);
Serial.print(" Plr 5: ");
Serial.print(player5);
Serial.print(" Plr 6: ");
Serial.print(player6);
Serial.println("");
//}
}

[Reply #1 on:]

You are running random separately for each player which probably isn't the right way to do it.

And you have to initialize the player counts to zero

int player1 = 0, player2 = 0 , .....

You'd probably be better off using an array for the player counts.

Pete

[Reply #2 on:]

I think you are right about if-else statements, they not doing what supposed to.
>  (random(1,3)==1) {player5++;}
Chances for player5 is much higher than for player1.
You should rewrite a code giving equals chances for player to withdraw a number, excluding already   taken out numbers

[Reply #3 on:]

You are running random separately for each player which probably isn't the right way to do it.

And you have to initialize the player counts to zero

int player1 = 0, player2 = 0 , .....

You'd probably be better off using an array for the player counts.

Pete

I was assuming the variables auto initialized to 0. If you don't initialize the variable, what value will it have?  Random?  I will try initializing to 0 later.  I cant see how drawing a new random number each time is a problem although it might not be the most elegant way.  I saw it as the easiest way to reduce the number of possibilities for each successive player.

I think you are right about if-else statements, they not doing what supposed to.
>  (random(1,3)==1) {player5++;}
Chances for player5 is much higher than for player1.
You should rewrite a code giving equals chances for player to withdraw a number, excluding already   taken out numbers

Chances for player 5 should be much higher, since player 5 will only get to draw on those times that players 1-4 do not draw '1'.  If players 1-4 draw 1 then that line of code shouldn't even be executed, right?

[Reply #4 on:]

Your variables are all automatics, and automatics do not auto-initialise.
If you want them to, make them global, or static.

[Reply #5 on:]

Pete and AWOL were right - initializing the variables fixed it.  Thanks guys.

[Reply #6 on:]

I saw it as the easiest way to reduce the number of possibilities for each successive player.

If that's what you want to do, that's OK - perhaps you are simulating Russian Roulette in which case your code is correct. But if what you are doing is randomly choosing one of the players each time, then you could do something like this:

Code:

  chance = random(1,7);
  if(chance == 1) {
    //do stuff for player 1
  }
  if(chance == 2) {
    // do stuff for player 2
  }
etc.

However, as I mentioned previously, in this case I would store the player counts in an array. It would make the code shorter.

Pete

[Reply #7 on:]

Code:

  chance = random(1,7);
  if(chance == 1) {
    //do stuff for player 1
  }
  if(chance == 2) {
    // do stuff for player 2
  }
etc.

Unless the value of chance can change (can a player defer to a different player?) I'd suggest using a switch statement rather than a long list of if statements. But even better would be to stick all the players into an array and have the same code deal with all of them.

[Reply #8 on:]

But even better would be to stick all the players into an array

Yes, that's what I said. I was leaving that as an exercise for the reader.

Pete

[Reply #9 on:]

Yes, it is something like Russian roulette I guess, without the blowing your head off part.  As each successive person draws and misses the next person has a better chance of winning.

I can appreciate the array ideas, but it runs plenty fast enough as is.  I was surprised how quickly the Arduino blows throughout 32000 runs of the code.  I think my memories of running BASIC on the Apple II and Commodore 64 led me to expect bad performance on the 8 bit Atmel.  I didn't account for the compiled code vs interpreted code and the faster clock speed on the Atmel chip.