views:

308

answers:

5

Ok so I have a dice throw app...

When I step through the code it functions normally and 'results' contains the correct number of throw results and they appear to be random, when I leave the code to run and do exactly the same thing it produces a set of identical numbers.

I'm sure this is a logical error I cannot see but fiddling with it for hours hasnt improved the situation, so any help is much appriciated. :)

    class Dice
{

    public int[] Roll(int _throws, int _sides, int _count)
    {
        Random rnd = new Random();
        int[] results = new int[_throws];
        // for each set of dice to throw pass data to calculate method
        for (int i = 0; i < _throws; i++)
        {
            int thisThrow = Calculate(_sides, _count);
            //add each throw to a new index of array... repeat for every throw
            results[i] = thisThrow; 
        }

        return results;
    }


    private int Calculate(int _sides, int _count)
    {
        Random rnd = new Random();
        int[] result = new int[_count];
        int total = 0;
        //for each dice to throw put data into result
        for (int i = 0; i < _count; i++)
        {
            result[i] = rnd.Next(1, _sides);
        }
        //count the values in result
        for (int x = 0; x < _count; x++)
        {
            total = total + result[x];
        }
        //return total of all dice to Roll method
        return total;
    }
}
A: 

Give the constructor Random a seed. That's the problem.

http://msdn.microsoft.com/en-us/library/aa329890%28VS.71%29.aspx

Random r = new Random(DateTime.Now.Millisecond);
Daniel A. White
Using Millisecond as a seed is even worse than the default of Ticks. The tick seed is fine in most cases its just as the others have said, do not re-create the Random class and therefore re-seed it back to the same value.
Robin Day
Ah. I am so used to the C-style rand functions.
Daniel A. White
+10  A: 

First mistake: Never use multiple instances of Random, use a single instance, and pass that along with the other parameters.

leppie
thanks modifying now :)
Yoda
damn quick draw!
Dead account
@Ian: I have seen this happen too many times not to spot it immediately :)
leppie
Legend thankyou!
Yoda
@leppie: Yup. It would be interesting to see how many SO questions involving RNGs were basically variants of this one...
Jon Skeet
+4  A: 

When you create "Random rnd = new Random();" it is seeded by the current time. When you debug your code (which takes time) it will be seeded differently each time.

Create 1 instance of Random, and reference that everywhere.

Dead account
+1  A: 

You're creating a random class every time you need to create a number. Doing this will give you the nutty results.

See here: FROM MSDN

This problem can be avoided by creating a single Random object rather than multiple ones.

To improve performance, create one Random object to generate many random numbers over time, instead of repeatedly creating a new Random objects to generate one random number.

E.g. create a private instance of Random...

Paul Sasik
+1  A: 

In addition to what has been mentioned before...

Use Random for things like dice, card games, choosing random images and so forth. If you ever need to create a random number for security sake, use System.Security.Cryptography.RandomNumberGenerator. This simple example shows creating a random integer.

        RandomNumberGenerator gen = RandomNumberGenerator.Create();
        byte[] myBytes = new byte[4];
        gen.GetBytes(myBytes);
        int myValue = (BitConverter.ToInt32(myBytes, 0));

DO NOT use this unless you have a security need. The performance is less than that of the Random class. I suppose you could use this to seed Random but that might be overkill.

EDIT: It occurred to me that I had never tested this. A quick performance test showed the following:

1,000,000 random numbers: RandomNumberGenerator: 2.6 seconds Random: .015 seconds.

So Random is about 150 times faster.

Bomlin
Two things. First, using a random number to seed a pseudo-random number generator does not actually get you more randomness; the output of the pseudo-RNG is still predictable.
Eric Lippert
Second, the performance of the crypto-strength RNG is gated not just on the math it has to do, but also on the ability of the machine to provide *entropy* for the RNG to operate on. RNGs don't invent entropy out of nothing, you know! That entropy has to come from somewhere, and entropy is every bit as limited a resource as, say, bandwidth, memory or processor time. You only get so much entropy per second; an attempt to use huge quantities of entropy will of course block until more entropy becomes available, just as an attempt to use processor or network bandwidth blocks.
Eric Lippert