ASP.NET MVC loop seems like not working

Question

I'm building a captcha like functionality and I have a method to draw random lines

private Graphics DrawRandomLines(Graphics g)
    {
        SolidBrush green = new SolidBrush(Color.Green);
        int count = 0;
        for (int i = 0; i < 20; i++)
        {
            g.DrawLines(new Pen(green, 2), GetRandomPoints());
            count++;
        }

        return g;
    }

The problem is only one line is painted on my graphics when I'm launching web app. But when I'm launching in debug mode loop is working and I'm getting all 20 lines painted.

Here is GetRandomPoints

private Point[] GetRandomPoints()
            {
                Random rand = new Random();
                Point[] points = { new Point(rand.Next(0, 100), 
                           rand.Next(0, 30)), 
                           new Point(rand.Next(0, 100), 
                           rand.Next(0, 30)) };
                return points;
            }

Can you show GetRandomPoints? Most likely it's due to thread safety, take a look at http://stackoverflow.com/questions/295900/system-random-keeps-on-returning-the-same-value — Lee Gary, Feb 19 '14 at 08:26

Andrei V · Accepted Answer · 2014-02-19T08:40:20.160

The problem seams to be that you're instantiating a new Random object for each of your 20 points. The time difference is to small and the "random" values are not, well, random enough. When you're debugging your app, you "pause" to inspect the value and the system clock advances, therefore a new Random object has a different seed from the previous. The bottom line is, your 20 lines were being drawn, just that they were having the same position, i.e. they were drawn on top of each other.

Change to this:

private Graphics DrawRandomLines(Graphics g)
{
    SolidBrush green = new SolidBrush(Color.Green);
    int count = 0;
    //instantiate only one time
    Random rand = new Random();
    //same here
    Pen greenPen = new Pen(green, 2);
    for (int i = 0; i < 20; i++)
    {
        g.DrawLines(greenPen, GetRandomPoints(rand));
        count++;
    }

    return g;
}

private Point[] GetRandomPoints(Random rand)
{
    Point[] points = { new Point(rand.Next(0, 100), 
                       rand.Next(0, 30)), 
                       new Point(rand.Next(0, 100), 
                       rand.Next(0, 30)) };
    return points;
}

When you create a `Random` object, it gets initialized with the current system clock. This initial value (seed) is used to generate (pseudo) random values. If you declare multiple `Random` objects fast enough, they all get initialized with the same seed and produce the same "random number" sequence. As a rule of thumb, `Random` objects should be declared once per class/app or at least once per "random number set" (e.g. your random points) and not for every random number. This works because the `Random` object gets initialized once and the succession of random numbers produces different values. — Andrei V, Feb 19 '14 at 08:50

score 1 · Answer 2 · answered Feb 19 '14 at 08:37

Every time you call GetRandomPoints, that function creates a new Random object, which will be seeded using the current clock time.

When you run this in Release mode, the 20 calls to GetRandomPoints happen so quickly that all twenty Random objects are seeded with the exact same value, and thus they all produce the same 'random' numbers. You are drawing twenty lines - they're just all identical!

When you run this in Debug mode, however, all the debug code that gets added slows things down enough that the Random objects get different seed values, and so the random numbers generated are different.

To fix this, you need to move from creating twenty different Random objects (which risks them all getting seeded identically) to creating a single Random object that you reuse. To do this, don't create the Random object inside GetRandomPoints - take it as a parameter instead, and create it in DrawRandomLines.

ASP.NET MVC loop seems like not working

2 Answers2