Problem with random dice rolling in C# console application

Question

I am writing dice roller program in C# console. I am giving two input

1. Enter the size of the dice and
2. Enter how times you want to play.

Suppose dice size is 6 and 10 times i have played.

Output is coming:
1 was rolled  2 times
2 was rolled  7 times
3 was rolled  8 times
4 was rolled  7 times
5 was rolled  4 times
6 was rolled  5 times

Total: 33 (its not fixed for every execution this no will be changed)

But my requirement was this total always should be no of playing times. Here I am playing 10 times so the total should be 10 not 33. It should happen for every new numbers... If I play 100 times sum or total should be 100 only not any other number. rest all will be remain same, in my programing not getting the expected sum. Pls somebody modify it. Here is my code:

Dice.cs:

public class Dice
{
    Int32 _DiceSize;
    public  Int32 _NoOfDice;     
    public Dice(Int32 dicesize)
    {
        this._DiceSize = dicesize;         
    }
    public string Roll()
    {
        if (_DiceSize<= 0)
        {
            throw new ApplicationException("Dice Size cant be less than 0 or 0");                 
        }
        if (_NoOfDice <= 0)
        {
            throw new ApplicationException("No of dice cant be less than 0 or 0");
        }
        Random rnd = new Random();
        Int32[] roll = new Int32[_DiceSize];
        for (Int32 i = 0; i < _DiceSize; i++)
        {
            roll[i] = rnd.Next(1,_NoOfDice);
        }
        StringBuilder result = new StringBuilder();
        Int32 Total = 0;
        Console.WriteLine("Rolling.......");
        for (Int32 i = 0; i < roll.Length; i++)
        {
            Total += roll[i];
            result.AppendFormat("{0}:\t was rolled\t{1}\t times\n", i + 1, roll[i]);
        }
        result.AppendFormat("\t\t\t......\n");
        result.AppendFormat("TOTAL: {0}", Total);
        return result.ToString();
    }
}
class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine("Enter no of dice size");
        int dicesize = Convert.ToInt32(Console.ReadLine());
        Console.WriteLine("How many times want to play");
        int noofplay=Convert.ToInt32(Console.ReadLine());
        Dice obj = new Dice(dicesize);
        obj._NoOfDice = noofplay;
        obj.Roll();
        Console.WriteLine(obj.Roll());           
        Console.WriteLine("Press enter to exit");
        Console.ReadKey();
    }
}

Answer 1

You are creating a new Random instance on each iteration. This is not a good thing as it will affect the uniform distribution of results. Keep the Random instance in a field instead of creating a new one every time.

public class Dice {
    private Random rnd = new Random();

    // ... don't create a new random in `Roll` method. Use `rnd` directly.
}

Answer 2

It looks to me like you're getting the math backwards... shouldn't it be:

// to capture just the counts
int[] roll = new int[_DiceSize];
for (int i = 0; i < _NoOfDice; i++)
{
    roll[rnd.Next(roll.Length)]++;
}

Or if you want the actual rolls:

// to capture individual rolls
int[] roll = new int[_NoOfDice];
for (int i = 0; i < _NoOfDice; i++)
{
    roll[i] = rnd.Next(_DiceSize) + 1; // note upper bound is exclusive, so +1
}

Answer 3

This is the full code you have to use, according to Mehrdad and Marc Gravell. Have fun.

  public class Dice
  {
    private Random rnd = new Random();

    Int32 _DiceSize;
    public Int32 _NoOfDice;
    public Dice(Int32 dicesize)
    {
      if (dicesize <= 0)
      {
        throw new ApplicationException("Dice Size cant be less than 0 or 0");
      }

      this._DiceSize = dicesize;
    }
    public string Roll()
    {

      if (_NoOfDice <= 0)
      {
        throw new ApplicationException("No of dice cant be less than 0 or 0");
      }
      // to capture just the counts
      int[] roll = new int[_DiceSize];
      for (int i = 0; i < _NoOfDice; i++)
      {
        roll[rnd.Next(roll.Length)]++;
      }
      StringBuilder result = new StringBuilder();
      Int32 Total = 0;
      Console.WriteLine("Rolling.......");
      for (Int32 i = 0; i < roll.Length; i++)
      {
        Total += roll[i];
        result.AppendFormat("{0}:\t was rolled\t{1}\t times\n", i + 1, roll[i]);
      }
      result.AppendFormat("\t\t\t......\n");
      result.AppendFormat("TOTAL: {0}", Total);
      return result.ToString();
    }
  }
  class Program
  {
    static void Main(string[] args)
    {
      Console.WriteLine("Enter no of dice size");
      int dicesize = Convert.ToInt32(Console.ReadLine());
      Console.WriteLine("How many times want to play");
      int noofplay = Convert.ToInt32(Console.ReadLine());
      Dice obj = new Dice(dicesize);
      obj._NoOfDice = noofplay;
      Console.WriteLine(obj.Roll());
      Console.WriteLine("Press enter to exit");
      Console.ReadKey();
    }
  }

Answer 4

First of all, the following for-loop is wrong:

for (Int32 i = 0; i < _DiceSize; i++)
{
   roll[i] = rnd.Next(1,_NoOfDice);
}

Obviously you switched _DiceSize and _NoOfDice. The correct loop would look like

for (Int32 i = 0; i < _NoOfDice; i++)
{
   roll[i] = rnd.Next(1,_DiceSize);
}

Because of that, the line

Int32[] roll = new Int32[_DiceSize];

has to be changed to

Int32[] roll = new Int32[_NoOfDice];

Maybe you should think about renaming these variables, so it's more clearly, which means what.

If you modify your code that way, you will mention that your analisys won't work that way you implemented it. Actually, what you are showing is the result of each throw, which is not what you want, if I understood you right.

UPDATE:

Sorry, I misunderstood you. You do want to show the result for every roll. So, why don't you just move the StringBuilder.AppendFormat to your "rolling-for-loop"?

UPDATE #2:

For me, the following Die-class works exactly the way you want it:

public class Die
{
    private int maxValue;
    private int numberOfRolls;
    private Random random;

    public Die(int maxValue, int numberOfRolls)
    {
        this.maxValue = maxValue;
        this.numberOfRolls = numberOfRolls;
        this.random = new Random();
    }

    public string Roll()
    {
        StringBuilder resultString = new StringBuilder();

        for (int i = 0; i < this.numberOfRolls; i++)
        {
            resultString.AppendFormat("Roll #{0} - Result: {1}" + Environment.NewLine, i + 1, this.random.Next(1, maxValue + 1));
        }

        return resultString.ToString();
    } 
}

Hope I could help you.