1

I have the following code :

int GetRandNumber()
{
    Random r = new Random();
    return r.Next() % 6 + 1;             
}

I am developing a windows store app. I have a textbox in which a single number from 1 to 6 is generated. For every number I have made a specific event. I need to make sure that every number is generated only once so that the events do not repeat.

tshepang
  • 12,111
  • 21
  • 91
  • 136
user3473574
  • 11
  • 3
  • Something of a nitpick in this situation, but using modulo (`%`) like this will not get you a fair distribution in some cases. E.g. see http://stackoverflow.com/questions/10984974/why-do-people-say-there-is-modulo-bias-when-using-a-random-number-generator; using a different overload of `Next` should get you a good, fair number within the bounds you need. – Tim S. Mar 28 '14 at 17:12

4 Answers4

7

Looks like you just need to shuffle numbers. I'd suggest to create an array of numbers from 1 to 6 and just shuffle the array. You can see some code here for how to shuffle an array/list.

Community
  • 1
  • 1
AD.Net
  • 13,352
  • 2
  • 28
  • 47
  • Sorry for bothering you, I still have not made it work. Look at the changes I made to the question. It is more clear now. – user3473574 Mar 28 '14 at 18:41
  • Basically you need 1 to 6 but randomly to bind to some event. My suggestion was to keep 1 to 6 in an array, shuffle the array so that when you read, it's randomized, and then read the array and bind your events. You'll not have duplicate events, the shuffling of the array should give you the numbers in random. – AD.Net Mar 29 '14 at 14:21
3

First of all you need to be careful with this implementation, if you call GetRandomNumber() multiple times very close together it will give you the same result. A better function would be

int GetRandNumber(Random r)
{
    return r.Next(1, 7); //This does the same as "r.Next() % 6 + 1" but removes bias.
}

//used like
Random rand = new Random();
foreach(var foo in bar)
{
    //The same instance of rand is used for each call instead of a new one each time.
    foo.SomeProp = GetRandNumber(rand);
 }

However that is completely separate from what you need to do, you should not be generating random numbers between 1 though 6. What you need to do is make a list of 1 though 6 then shuffle the list, not use random numbers at all (well you will likely use them during the shuffle but that is a implementation detail)

List<int> numbers = new List<int>();
for(int i = 1; i <= 6; i++)
{
    numbers.Add(i);
}

MyShuffleListFunction(numbers);

There are plenty of examples on this site on how to make a shuffle function.

Community
  • 1
  • 1
Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
0

Maybe I'm wrong, but as I understand you want something like this as an output:

344213266125

To achieve this you should keep track which numbers are already generated, and stop if all has been "rolled" at least once. So let's have a bool array initialized to 6 false values, and after each random number generation (after each roll) set the array's corresponing element to true. Check if there are any false values in the array, if not then "roll again".

Also, you might consider generating the numbers according to the other answers, as they are of course right: this is not an ideal way.

Update

Okay, based on your question's update I also update my pseudocode (or maybe I should call this prose): Keep track of the already "rolled" numbers (as above). When generating a new number check if it has been already generated or not. If it's been rolled before, then roll again, until you roll a new one: then update the array accordingly. Also be sure to check if there is a valid number or all has been rolled before at the beginning of the roll...

But now it seems for me that what you really are looking for is simply a shuffle, which has been proposed in other answers.

qqbenq
  • 10,220
  • 4
  • 40
  • 45
  • Sorry for bothering you, I still have not made it work. Look at the changes I made to the question. It is more clear now. – user3473574 Mar 28 '14 at 18:40
  • In that case - based on the suggestion I gave in the answer - you should update the array after each "roll": see if the number you rolled is already rolled once, and if so, then roll again, if not, then set the flag in the array and return with that number. – qqbenq Mar 28 '14 at 18:45
  • like this? `if (generated.Text.Contains(GetRandNumber().ToString())) { numbers.Visibility = Windows.UI.Xaml.Visibility.Collapsed; } numbers.Text=GetRandNumber().ToString();` – user3473574 Mar 28 '14 at 19:04
  • No, this doesn't seem right. But you should add your existing code to the question - or maybe create a new one, as the problem seems to be different than the original one. – qqbenq Mar 28 '14 at 19:07
  • `int GetRandNumber() { Random r = new Random(); return r.Next(1,7); } private void Button_Click_1(object sender, RoutedEventArgs e) { if (generated.Text.Contains(GetRandNumber().ToString())) { numbers.Visibility = Windows.UI.Xaml.Visibility.Collapsed; } numbers.Text=GetRandNumber().ToString(); } private void numbers_TextChanged(object sender, TextChangedEventArgs e) { generated.Text = numbers.Text + generated.Text; }` – user3473574 Mar 28 '14 at 19:21
0

Perhaps this will work for you. The approach is to record the random results in a List and to skip the output if the results already exists in the list.

class Program
{
    static int tmp;

    static void Main(string[] args)
    {
        List<int> alreadyFired = new List<int>();                   

        while (alreadyFired.Count != 6)
        {

            Random rng = new Random();

            int diceresult = rng.Next(1,7);

            foreach (var item in alreadyFired)
            {
                if (item == diceresult)
                {
                    tmp = item;
                }
            }

            if (!(tmp == diceresult))
            {
                alreadyFired.Add(diceresult);                 
                Console.WriteLine(diceresult);
            }
        }

        Console.WriteLine("no events left");
        Console.ReadKey();
    }
}
oldsport
  • 993
  • 2
  • 14
  • 37