Dice returning 0 and no rolls

Question

playerDice = new Dice();
int playerDiceNo = playerDice.getfaceofDie();
MessageBox.Show("Your roll" + playerDiceNo);

compDice = new Dice();
int compDiceNo = compDice.getfaceofDie();
MessageBox.Show("Computers roll:" + compDiceNo);

above is my method for when the roll button is clicked. Below is my dice class:

class Dice
{
    private int faceofDie;
    public void rollDice()
    {
        Random rollDice = new Random();
        faceofDie = rollDice.Next(1, 7);          
    }
    public int getfaceofDie()
    {
        return faceofDie;
    }
}

I have stated my variables for compDice and playerDice as :

Dice compDice;
Dice playerDice;

I can't seem to figure out why it's returning 0 for both rolls over & over. can anyone help?

possible duplicate of [Random number generator only generating one random number](http://stackoverflow.com/questions/767999/random-number-generator-only-generating-one-random-number) — Alexei Levenkov, May 02 '13 at 16:10
You code sample shows wrong usage of `Random` - please see duplicate question - if you still see the problem updated your sample and provide more details. — Alexei Levenkov, May 02 '13 at 16:11

score 5 · Accepted Answer · answered May 02 '13 at 15:55

I can't seem to figure out why it's returning 0 for both rolls over & over. can anyone help?

You never call rollDice(), so the faceofDie variable is never set, and has it's default value of 0.

playerDice = new Dice();
playerDice.rollDice(); // Add this
int playerDiceNo = playerDice.getfaceofDie();
MessageBox.Show("Your roll" + playerDiceNo);

A better approach would be to roll the dice the first time in the constructor, and to not keep creating new Random instances:

class Dice
{
    private static Random diceRoller = new Random();

    private int faceofDie;

    public Dice()
    {
        this.RollDice(); // Roll once on construction
    }

    public void RollDice()
    {   
        lock(diceRoller) 
            faceofDie = diceRoller.Next(1, 7);          
    }

    public int FaceOfDie
    {
        get { return faceofDie; }
    }
}

The static Random instance will prevent multiple dice implemented at the same time from getting the same seed (as they'll all share a single random), which will help keep your results more consistent. This also moves to standard C# conventions, and would be used like:

playerDice = new Dice();
int playerDiceNo = playerDice.FaceOfDie;
MessageBox.Show("Your roll" + playerDiceNo);

compDice = new Dice();
int compDiceNo = compDice.FaceOfDie;
MessageBox.Show("Computers roll:" + compDiceNo);

@user2326995 My code, as written above, will return 2 different numbers between 1 and 6 (inclusive) - just tested to verify. — Reed Copsey, May 02 '13 at 16:01
Sorry, my mistake, my computer wasn't showing the second lot of code. Thankyou so much for your help this works perfectly. — user2326995, May 02 '13 at 16:04

Dice returning 0 and no rolls

1 Answers1