Re: Random.next(min,max) - C# Discussion Boards

9-Apr-10 0:42

Hey,
I'm using that method, but almost always (all executions) i get the same generated random int,
my the min is always 0, and the max moves from 1 to 3 (vector index)
no, almost always i get 0, even if max is 3 (5 executions and it's always 0) the same thing happens when max is 2...

the initialization of random object is with default constructor

new Random();

any idea how to make it be more statistical ?

thanks

p.s
I know that
default constructor works with Clock ticks
(from MSDN)
by default, the parameterless constructor of the Random class uses the system clock to generate its seed value

Luc Pattyn9-Apr-10 0:55

Getting an article published on CodeProject should be easier and faster for Bronze and Silver authors.

9-Apr-10 0:55

you are aware of this:

maxValue
The exclusive upper bound of the random number returned.

?

if so, show actual code and values.

Smile | :)

Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]

igalep1329-Apr-10 1:02

9-Apr-10 1:02

for (int i = 0; i < zeroFrequencyLetterList.Count && decrypted < 25; i++)
                       {
                           bool flag = true;
                           Random r = new Random();
                           int randomed;
                           do
                           {
                               randomed = r.Next(0, zeroFrequencyLetterList.Count);//<--- here i get almost always '0', no meter how big the ArrayList, almost always it means that one in X executions i get different answer than '0'

                               if (encryptionKey[zeroFrequencyLetterList[randomed]] != ' ')
                                   flag = false;
                               else
                                   flag = true;
                           } while (!flag);

                           encryptionKey[zeroFrequencyLetterList[i]] = abcLetters[randomed];
                           abcLetters.RemoveAt(randomed);
                           zeroFrequencyLetterList.RemoveAt(i);
                           decrypted++;
                       }

Luc Pattyn9-Apr-10 1:08

Getting an article published on CodeProject should be easier and faster for Bronze and Silver authors.

9-Apr-10 1:08

OK.

1.
move Random r = new Random(); outside the for loop, I see no reason to have it inside; normally all you need is a single Random as a class member; and you might make it static.

2.
you are looping the do-while until encryptionKey[zeroFrequencyLetterList[0] is a space; so having random=0 is popular, and even more so when it is the only space in the collection.

Smile | :)

Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]

igalep1329-Apr-10 1:18

9-Apr-10 1:18

as i already said, usually i've 2-3 empty spaces in collection
but i always get the first place ...
and my description is about application execution, i mean that each time i execute the application, and not during the running time

and your first advice has changed nothing Frown | :(

David Skelly9-Apr-10 2:05

9-Apr-10 2:05

I tried this using min 0 and max 10. In my case, it seemed to like the number 3 quite a lot and 0 didn't come up much at all. Interestingly, when I reused the same instance of Random rather than creating a new one each time, I seemed to get a better spread of numbers (purely subjective since humans aren't very good at judging how random a sequence truly is).

The plain truth is that Random isn't particularly random. If you need random numbers that are closer to being genuinely random, use one of the cryptographic random number generators. But, they are quite slow to initialise so in that case you should definitely not create a new one each time through the loop.

PIEBALDconsult9-Apr-10 3:34

PIEBALDconsult

9-Apr-10 3:34

None of that matters as long as you keep making new instances of Random -- do not instantiate Random inside the for loop!

Luc Pattyn9-Apr-10 1:56

Getting an article published on CodeProject should be easier and faster for Bronze and Silver authors.

9-Apr-10 1:56

OK, can you provide the declaration and the content (just before entering the for loop) of zeroFrequencyLetterList? If it varies, just a typical case; if complex, a simplified one that still makes you unhappy about Random.Next

Smile | :)

Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]

David Skelly9-Apr-10 2:29

Re: Random.next(min,max) [modified]

9-Apr-10 2:29

Off topic, so feel free to ignore this, but your loop count looks a bit odd to me. You increment i and test for it being less than zeroFrequencyLetterList.Count. But you remove an entry from the list each time through the loop. So, this loop will drop out before it has processed the entire list. Here is what happens if you have 10 entries in the list:

The loop will stop at this point, so although you have 10 entries in the list you will only process 5 of them.

Not only that, but you will skip entries in the list. First time through, i is 0 so you remove the entry at index 0, so the entry that was at index 1 is now at index 0, and the entry at index 2 is now at 1. Next time through, i is 1 so you now pick up the entry at index 1 which means you have skipped the entry which was previously at index 1 but is now at index 0. (That's a bit confused, but if you think about it you'll see what I mean.)

That may be what you want, but it looks a bit strange to me.

igalep1329-Apr-10 2:42

9-Apr-10 2:42

you absolutely right,
but luckily i had there another condition && decrypted < 25 therefor it stopped the loop just on time
but you right, I fixed it

Random r = new Random();
                       int leftLetters = zeroFrequencyLetterList.Count;
                       for (int i = 0; i < leftLetters && decrypted < 25; i++)
                       {
                           bool flag = true;
                           int randomed;
                           do
                           {
                               randomed = r.Next(0, zeroFrequencyLetterList.Count);

                               if (encryptionKey[zeroFrequencyLetterList[randomed]] != ' ')
                                   flag = false;
                               else
                                   flag = true;
                           } while (!flag);

                           encryptionKey[zeroFrequencyLetterList[0]] = abcLetters[randomed];
                           abcLetters.RemoveAt(randomed);
                           zeroFrequencyLetterList.RemoveAt(0);
                           decrypted++;
                       }

modified on Friday, April 9, 2010 8:50 AM

David Skelly9-Apr-10 2:57

9-Apr-10 2:57

I still don't think this works because you are removing an entry from the list each time through. Here is what happens:

i = 0;  list = 0, 1, 2, 3;  list[i] = 0;  remove i
i = 1;  list = 1, 2, 3;     list[i] = 2;  remove i
i = 2;  list = 1, 3;        list[i] = oops!

igalep1329-Apr-10 5:36

9-Apr-10 5:36

i'm not removing at i's place, i'm removing the first place of the collection

Luc Pattyn9-Apr-10 2:18

Getting an article published on CodeProject should be easier and faster for Bronze and Silver authors.

9-Apr-10 2:18

I ran some tests with this code:

public override void Test(int arg) {
    int n=10;
    int max=1000000;
    if (arg==2) { n=10; max=1000; }
    if (arg==3) { n=3; max=100; }
    Random r=new Random();
    int[,] hist=new int[n,n];
    int prev=0;
    for (int i=0; i< max; i++) {
        int j=r.Next(0, n);
        hist[prev, j]++;
        prev=j;
    }
    for (prev=0; prev< n; prev++) {
        string s="";
        for (int j=0; j< n; j++) {
            s+=hist[prev, j].ToString().PadLeft(8);
        }
        log(s);
    }
}

and don't see anything wrong with Random, not even over a small [min,max) range.

Smile | :)

Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]

modified on Friday, April 9, 2010 8:41 AM

riced9-Apr-10 2:22

riced

9-Apr-10 2:22

Not sure what your problem is - it works as expected for me and as Luc says (and he's not often wrong - if ever. Smile | :)

)
Here's a little console app that does what I think you're doing.
Note the bogus variables - you can adjust them to see what happens.
(Gets rid of unnecessary flag but logic is the same. Smile | :)

)

using System;
using System.Linq;

namespace ConsoleApplication1
{
   class Program
   {
      static void Main(string[] args)
      {
         Tester();
         Console.ReadLine();
      }
      public static void Tester()
      {
         //Bogus variables to emulate code
         int[] zeroFrequencyLetterList = { 1, 2, 3, 4, 0, 5, 10, 9, 8, 7, 6, 5, 10, 2, 9, 7, 0, 6 };
         int decrypted = 0;
         char[] encryptionKey = { 'a', 'b', ' ', 'c', 'd', 'e', ' ', ' ', 'f', ' ', 'g' };

         Random r = new Random();
         for (int i = 0; i < zeroFrequencyLetterList.Count() && decrypted < 25; i++)
         {
            int randomed;
            char aLetter;
            do
            {
               randomed = r.Next(0, zeroFrequencyLetterList.Count());
               aLetter = encryptionKey[zeroFrequencyLetterList[randomed]];
               Console.WriteLine("{0,3}: {1,3} -> {2}", i, randomed, aLetter ); 
            } while (aLetter == ' ');

            //    encryptionKey[zeroFrequencyLetterList[i]] = abcLetters[randomed];
            //    abcLetters.RemoveAt(randomed);
            //    zeroFrequencyLetterList.RemoveAt(i);
            decrypted++;
         }
      }
   }
}

Regards
David R
---------------------------------------------------------------
"Every program eventually becomes rococo, and then rubble." - Alan Perlis
The only valid measurement of code quality: WTFs/minute.

David Skelly9-Apr-10 2:34

9-Apr-10 2:34

You create r outside the loop and reuse it. The OP created a new Random instance every time. It is possible that Random has some sort of bias to the first number which it generates, depending on the internal algorithm it uses. It certainly seemed that way when I tried it (completely unscientific, I admit).

ragnaroknrol9-Apr-10 6:18

ragnaroknrol

9-Apr-10 6:18

I am pretty sure the seed for the random number is being intialized the same or close enough to skew the results thanks to being in that loop.

Random is actually fairly predictable if you know the seed and the range.
It's how they make those wonderful authenticator programs and keyfabs.
Synch it with one on another site and both will have the exact same number every time.

Check out the seeds comments[^]

Luc Pattyn9-Apr-10 2:39