1

This section of code is misbehaving. I want to draw a user defined grid of circles with each a random color from a set list of 7 colors. The random number generator is supposed to do this. The grid of circles gets drawn fine, its the colors that are giving me grief. I seem to get a max of two colors per grid, with the first dozen being one color and the rest being a second color. Its strange, as the code should cycle through the color generator, then draw one circle and repeat. Please help me find the troublesome lines, Spent far too long trying by myself!

Ignore references to JEWEL_HEIGHT and similar, they are just variable names relevant to the program.

              int columns = int.Parse(textBoxColumns.Text);
            int rows = int.Parse(textBoxRows.Text);


            for (int y = 0; (y < rows * 20); y += JEWEL_HEIGHT)
            {
                for (int x = 0; (x < columns * 20); x += JEWEL_WIDTH)
                {


                    Color brushColor = (Color.Red);
                    Random randGen = new Random();
                    int randColor = randGen.Next(7);
                    if (randColor == 0)
                        brushColor = (Color.Red);
                    else if (randColor == 1)
                        brushColor = (Color.Orange);
                    else if (randColor == 2)
                        brushColor = (Color.Yellow);
                    else if (randColor == 3)
                        brushColor = (Color.Green);
                    else if (randColor == 4)
                        brushColor = (Color.Blue);
                    else if (randColor == 5)
                        brushColor = (Color.Indigo);
                    else if (randColor == 6)
                        brushColor = (Color.Violet);

                    Graphics paper = pictureBoxJewels.CreateGraphics();
                    SolidBrush brush = new SolidBrush(brushColor);

                    paper.FillEllipse(brush, x, y, JEWEL_WIDTH, JEWEL_HEIGHT);
user1118321
  • 25,567
  • 4
  • 55
  • 86
poop
  • 11
  • 2
  • Related: http://stackoverflow.com/questions/767999/random-number-generator-only-generating-one-random-number – RMalke Apr 01 '13 at 08:16

3 Answers3

5

Not sure because i cannot test it but Random randGen = new Random(); should not be inside the for loops . Put it before the first for loop and keep the randGen.Next(7); inside the loop as it is .

isioutis
  • 304
  • 2
  • 13
  • Brilliant, works perfect. 10 internet points to you good sir! – poop Apr 01 '13 at 08:32
  • Thank you . I am glad it helped u . If u need to understand more about why this happened check http://msdn.microsoft.com/en-us/library/system.random.aspx it has to do with the seeding on the initialization of Random() . – isioutis Apr 01 '13 at 08:46
0

you initializing randgen every time. here Random() not really ramdom it follow some logic.. and its time dependent cause its executing fast you may not get different number cause it may get same tick count in system.

i Edited my answer

Civa
  • 2,058
  • 2
  • 18
  • 30
  • Still valid, thank you! – poop Apr 01 '13 at 08:33
  • @Civa, "every time you initializing it done from starting. initialize out side loop" that's not true, Random uses Environment.TickCount in default constructor, but since inner loop runs very fast, TickCount may not change – illegal-immigrant Apr 01 '13 at 08:51
  • Thanks :) i dont know that thing . i just wrote this by reference of http://msdn.microsoft.com/en-us/library/system.random.aspx . --> its follows Pseudo-random algorithm. The random number generation starts from a seed value. If the same seed is used repeatedly, the same series of numbers is generated – Civa Apr 01 '13 at 09:02
0

Here's Random default constructor (.NET 4.5, decompiled):

public Random()
      : this(Environment.TickCount)
    {
    }

Hint: with high probability you will end up using same seed, move Random creation before loops

Some additional info:

The random number generation starts from a seed value. If the same seed is used repeatedly, the same series of numbers is generated. One way to produce different sequences is to make the seed value time-dependent, thereby producing a different series with each new instance of Random. By default, the parameterless constructor of the Random class uses the system clock to generate its seed value, while its parameterized constructor can take an Int32 value based on the number of ticks in the current time.

But since your inner loop runs very fast, and you create new instance on each iteration, there is possibility that TickCount may not change

illegal-immigrant
  • 8,089
  • 9
  • 51
  • 84