2

I'm using the following code to generate 4 random numbers, but each time I try to get the they're coming up as "system.random" in the textblock. Help anyone? Thanks!! :)

private void Button_Click_1(object sender, RoutedEventArgs e)        
{           
    Random dc1 = new Random();
    int dealCard1 = dc1.Next(52);

    Random dc2 = new Random();
    int dealCard2 = dc2.Next(52);

    Random pc1 = new Random();
    int playerCard1 = pc1.Next(52);

    Random pc2 = new Random();
    int playerCard2 = pc2.Next(52);

    txtDC1.Text = Convert.ToString(dc1);
    txtDC2.Text = Convert.ToString(dc2);
    txtPC1.Text = Convert.ToString(pc1);
    txtPC2.Text = Convert.ToString(pc2);
}
BradleyDotNET
  • 60,462
  • 10
  • 96
  • 117
will36
  • 39
  • 2

6 Answers6

3

First off, you only need one random instance:

Random dc1 = new Random();
int dealCard1 = dc1.Next(52);
int dealCard2 = dc1.Next(52);
int playerCard1 = dc1.Next(52);
int playerCard2 = dc1.Next(52);

The errors are because you're not reporting the numbers:

txtDC1.Text = Convert.ToString(dealCard1);
txtDC2.Text = Convert.ToString(dealCard2);
txtPC1.Text = Convert.ToString(playerCard1);
txtPC2.Text = Convert.ToString(playerCard2);
Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • Further, one shouldn't be spinning up a new RNG (or 4!) every time a button is clicked. Instantiate just one RNG at startup and use it over the life of the app. – Nicholas Carey May 07 '14 at 23:42
  • 1
    @NicholasCarey While I agree, doing it on a button click will work fine, since that won't happen enough times to get the same seed. – Reed Copsey May 07 '14 at 23:43
1

You are converting the random number generator, not the random number.

Your random number generators are dc2, pc1, and pc2. Your random numbers are dealCard2, playerCard1, and playerCard2.

From here you should be able to solve the problem.

tenfour
  • 36,141
  • 15
  • 83
  • 142
1

Taking just one for simplicity:

Random dc1 = new Random();
int dealCard1 = dc1.Next(52);
txtDC1.Text = Convert.ToString(dc1);

Here you've converted dc1, which is a Random object to a string, not dealCard1, which is the random number.

txtDC1.Text = Convert.ToString(dealCard1);

And so on.

(Note also, if the idea here is to pick for out of 52 cards, as with a playing card deck, that you aren't checking you don't have more than one of the exact same card, which may or may not be a problem).

Jon Hanna
  • 110,372
  • 10
  • 146
  • 251
0

You are putting the Random object itself into the textblock, hence "System.Random".

Your code should be:

txtDC1.Text = Convert.ToString(dealCard1);
txtDC2.Text = Convert.ToString(dealCard2);
txtPC1.Text = Convert.ToString(playerCard1);
txtPC2.Text = Convert.ToString(playerCard2);

You also shouldn't recreate the Random instance each time. Just use one (ideally as a class level variable). This prevents re-seeding the Random object with (nearly) the same seed value (which is based on the system time). As is, each of your "random" numbers could very well be identical!

BradleyDotNET
  • 60,462
  • 10
  • 96
  • 117
0

First of all you're converting the wrong variable, what you want is

txtDC1.Text = Convert.ToString(dealCard1);
txtDC2.Text = Convert.ToString(dealCard2);
txtPC1.Text = Convert.ToString(playerCard1);
txtPC2.Text = Convert.ToString(playerCard2);

Also try to avoid creating a new instance of random every time you call the click event. Consider creating one random instance that can be reused.

If you must create a new instance at every event, then consider adding a seed to the random, as quick succession of events may cause the Random instance to be seeded with the same seed and thus causing repeated values.

jpmnteiro
  • 747
  • 1
  • 13
  • 23
  • 1
    You don't actually need to reseed the random (it seeds based on the system time) when you create it. Creating multiple instances actually causes a problem, because each new instance gets the same seed. – BradleyDotNET May 07 '14 at 23:45
  • True indeed, just looked at the decompiled Random. I'll update the answer to avoid adding to the confusion. Thanks for the heads up. – jpmnteiro May 07 '14 at 23:49
0

I had trouble with this before and the error that I had was that I had too many random instance. So by looking at your code I see you got more than one. I would alter the coding to have only one random instance.

Yuliam Chandra
  • 14,494
  • 12
  • 52
  • 67
LaTachuela
  • 11
  • 5