Reputation: 2125
I'm working on a roulette wheel class, it should function more or less like a regular roulette wheel where certain numbers can take up a larger portion of the roulette wheel and thus have a higher probability of being chosen.
So far it has passed the more basic unit tests, that is, programatically it works, I can create a roulette wheel and fill it with a bunch of generic values and it will do just that.
However when it comes to my probability testing, I decided to try it out as a 6-sided die, after 10,000,000 trials it should generate an average die roll of about 3,5 unfortunately it doesn't even come close to that the average after 10,000,000 trials is about 2,9 so I am guessing there is a weakness in my number selection somewhere? I have posted unit tests and actual code below:
public class RouletteNumber<T>
{
public readonly T Number;
public readonly int Size;
public RouletteNumber(T number, int size)
{
this.Number = number;
this.Size = size;
}
public static RouletteNumber<T>[] CreateRange(Tuple<T, int>[] entries)
{
var rouletteNumbers = new RouletteNumber<T>[entries.Length];
for (int i = 0; i < entries.Length; i++)
{
rouletteNumbers[i] = new RouletteNumber<T>(entries[i].Item1, entries[i].Item2);
}
return rouletteNumbers;
}
}
public class RouletteWheel<T>
{
private int size;
private RouletteNumber<T>[] numbers;
private Random rng;
public RouletteWheel(params RouletteNumber<T>[] rouletteNumbers)
{
size = rouletteNumbers.Length;
numbers = rouletteNumbers;
rng = new Random();
//Check if the roulette number sizes match the size of the wheel
if (numbers.Sum(n => n.Size) != size)
{
throw new Exception("The roulette number sections are larger or smaller than the size of the wheel!");
}
}
public T Spin()
{
// Keep spinning until we've returned a number
while (true)
{
foreach (var entry in numbers)
{
if (entry.Size > rng.Next(size))
{
return entry.Number;
}
}
}
}
}
[TestMethod]
public void DiceRouletteWheelTest()
{
double expected = 3.50;
var entries = new Tuple<int, int>[]
{
Tuple.Create(1, 1),
Tuple.Create(2, 1),
Tuple.Create(3, 1),
Tuple.Create(4, 1),
Tuple.Create(5, 1),
Tuple.Create(6, 1)
};
var rouletteWheel = new RouletteWheel<int>(RouletteNumber<int>.CreateRange(entries));
var results = new List<int>();
for (int i = 0; i < 10000000; i++)
{
results.Add(rouletteWheel.Spin());
}
double actual = results.Average();
Assert.AreEqual(expected, actual);
}
}
Upvotes: 0
Views: 2012
Reputation: 3315
Maybe i'm not understanding it correctly but I'm guessing the problem lies here :
while (true)
{
foreach (var entry in numbers)
{
if (entry.Size > rng.Next(size))
{
return entry.Number;
}
}
}
You're calculating the rng.Next each time you do the if check.
So the first number has a 1 in 6 chance of being taken. Number 2(the next entry in numbers) then has a 2 in 6 chance of being taken(a new number is rendered between 1 and 6). But since you always start from the first you will eventually have more lower numbers.
I also don't see the need for the while(true) in a normal random generator.
I'm guessing this could would work and look like your current code :
var rndValue = rng.Next(size);
foreach (var entry in numbers)
{
if (entry.Size > rndValue)
{
return entry.Number;
}
}
Upvotes: 1
Reputation: 109567
When you call Random.Next(n)
it generates a random number between 0 and n-1, not between 0 and n.
Did you account for that?
In fact for a 6-sided die, you would want to call Random.Next(1, 7)
Upvotes: 3