tags:

views:

58

answers:

3

The following code snippet only returns in grayscale. I have been troubled by this for the majority of the day. Any thoughts on the reason why?

using System.Drawing;
    private Graphics _g;

    public Form1()
    {
      InitializeComponent();
      _g = pictureBox1.CreateGraphics();
    }

    private void x()
    {
      System.Drawing.Rectangle r = CreateCircle(e);
      SolidBrush brsh = ChooseFillColor();
      _g.FillEllipse(brsh, r);
    }
    private SolidBrush ChooseFillColor()
    {
      return new SolidBrush(Color.FromArgb(RandomNumber(255), RandomNumber(255), RandomNumber(255)));
    }
    private int RandomNumber(int max)
    {
      Random random = new Random();
      return random.Next(max);
    }
+4  A: 

You initialize a new Random object every time. Since it uses the current time by default as its seed, and you call it so quickly that the time doesn't change (on Windows, the timer's accuracy is about 15 milliseconds), you always get the same random number. Create a static Random object in your class instead.

EDIT: Also, consider making your RandomNumber method static as well, or put it (along with the static Random object) in a static class.

Francis Gagné
@Francis - good points, I say drop the `RandomNumber()` method altogether since it's no longer necessary - just call `rng.Next(256)` when you need it (also note 256 is an exclusive upper-bound so 255 would be the highest value returned)
John Rasch
+1  A: 

Try making your instance of Random a static member. I've run into the same sorts of problems before with random.Next(...) not returning expected values, and that fixed the problem.

Pwninstein
+2  A: 

Your calls to RandomNumber is happening so fast that the RNG is being seeded with the same value, thus you're getting the same integer for each call... this means all your colors will have the same R, G, and B values (i.e. grayscale).

Move your Random object outside of the method into a static member:

static Random rng = new Random();

private int RandomNumber(int max)
{
   return rng.Next(max);
}
John Rasch