-2

I have a program that rolls two die. The die are objects created from the Dice class. The Dice class contains a method that returns a random number between 1 and the number of sides that the dice contains. Like so:

class Dice
{
    //field
    private int sides;
    private Random rand = new Random();

    //Constructor omitted for brevity

    //public method to roll the dice.
    public int rollDie()
    {
        return rand.Next(sides) + 1;
    }
}

This works just fine for one dice. However, when I create two die and run both of their rollDie methods at the same time, I always receive the same number between the two die objects. For example:

Dice dice1 = new Dice(6); //created two die with 6 sides each.
Dice dice2 = new Dice(6);

dice1.rollDie(); //problem functions
dice2.rollDie();

those two functions will always return the same number, i.e. (1,1), (2,2), etc... never parting from this pattern.

From my research, I have concluded that the random object is creating a random number from the system's time and because the two methods are performed at nearly the same exact moment, they are returning a random number from the same seed. This is obviously a problem because it is far from reality.

The only solution I have tried so far, and the only one I can think of, was to define my own seed for each Dice object to be used in my Random object, but that seems to return the same number each time the rollDie method is called between each time the program is run (i.e. it will return (2, 5), (1, 3) every time the program is run with a seed of 1 and 2, respectively).

The question I have is if there is any way to prevent the two objects from returning the same number as the other?

Phillip Radke
  • 3
  • 4
  • 1
    Same seed, same number sequence. That's precisely why it's there - to allow reproducing the same sequence. *Don't* provide a seed at all. The [default is to use the CPU's tick count](https://github.com/Microsoft/referencesource/blob/master/mscorlib/system/random.cs#L52). – Panagiotis Kanavos Jan 16 '18 at 16:38
  • Please dont omit the constructor, it seems like thats your issue (assuming you are setting the seed there) – maccettura Jan 16 '18 at 16:44
  • 1
    `From my research, I have concluded...` your research ought to include [MSDN](https://msdn.microsoft.com/en-us/library/system.random(v=vs.110).aspx): *because the clock has finite resolution, using the parameterless constructor to create different Random objects in close succession creates random number generators that produce identical sequences of random numbers.* or alternatively, several hundred SO posts – Ňɏssa Pøngjǣrdenlarp Jan 16 '18 at 16:44
  • @PanagiotisKanavos: Isn't it an upper limit? https://msdn.microsoft.com/en-us/library/zd1bc8e5.aspx – David Jan 16 '18 at 16:44
  • @David I mistook the Dice constructor for the Random constructor. – Panagiotis Kanavos Jan 16 '18 at 16:46
  • 1
    Wouldn't using a single static instance fix all of this? – maccettura Jan 16 '18 at 16:46
  • @maccettura Random isn't thread-safe. – Panagiotis Kanavos Jan 16 '18 at 16:47
  • @Phillip: Treat the randomizer as a dependency. Instead of having the object internally instantiate the dependency, have it require that one be supplied. Consuming code can create a single instance of `Random` and provide it to the two objects. – David Jan 16 '18 at 16:47
  • @PanagiotisKanavos The code isn't using multiple threads. – Servy Jan 16 '18 at 16:48
  • @maccettura, I was not providing a seed at all in this particular code snippet, I was simply using the default seed, which as Plutonix pointed out, is using the CPU clock. – Phillip Radke Jan 16 '18 at 16:48
  • Pass a different seed value to each RNG. You could generate them using *another* RNG, or you could simply pass two different values, eg 1 and 2. The number sequences will be different. If you want a *strong* RNG, use one of the cryptographic classes. Those sequences aren't reproducible though – Panagiotis Kanavos Jan 16 '18 at 16:49
  • @Servy the code isn't about using a single RNG either. It's about generating two different sequences. – Panagiotis Kanavos Jan 16 '18 at 16:50
  • @PhillipRadke ah ok. Panagiotis had me confused for a minute. Use a static instance (wrap in lazy if you’re worried about thread safety). Consider even dependency injecting the random as well. – maccettura Jan 16 '18 at 16:51
  • 1
    Why do you need 2 `Dice` objects? Its nothing but a thin wrapper for the RNG, just call it 2ce as often. The results can map to 2 different player classes or outputs – Ňɏssa Pøngjǣrdenlarp Jan 16 '18 at 16:53
  • @Plutonix eventually, I want to have the two dice objects possess different attributes, i.e. different amounts of sides, colors, etc... – Phillip Radke Jan 16 '18 at 17:03
  • @PhillipRadke Use properties, not fields since I assume you will want to know the properties of each dice right? Also, if you use the static rnd like I said you [wont have the problems](https://dotnetfiddle.net/hDEmqt) anymore. – maccettura Jan 16 '18 at 17:08
  • @maccettura The static instance did fix the issue, so thank you for that. If you've got a minute, could you please explain why that worked? – Phillip Radke Jan 16 '18 at 18:05
  • @PhillipRadke static means its a class level instance, not an instance level instance. In other words, no matter how many Dice instances you have, they all share that same static Random instance. This eliminates your problem because youre no longer creating Random instances in quick succession (which gives you two Randoms with the same internal seed since its based on time). My solution is not thread safe however but you wont need to worry about that since your code is not utilizing multi threading. – maccettura Jan 16 '18 at 18:17

1 Answers1

-1

The two dice objects are being created so close together they're getting the same seed. If you stick a delay between them (e.g. Thread.Sleep(2000)) you'll get different values.

Craig W.
  • 17,838
  • 6
  • 49
  • 82