C# Random of cordinates is linear

Question

My code is to generate random cordinates of lat and long within a bound:

        Random lastLat = new Random();

        Random lastLon = new Random();

        for (int i = 0; i < 50; i++)
        {
            int lat = lastLat.Next(516400146, 630304598); //18.51640014679267 - 18.630304598192915

            int lon = lastLon.Next(224464416, 341194152); //-72.34119415283203 - -72.2244644165039

            SamplePostData d0 = new SamplePostData();
            d0.Location = new Location(Convert.ToDouble("18." + lat), Convert.ToDouble("-72." + lon));
            AddPushpin(d0);

        }

My output looks like this: http://img263.imageshack.us/img263/7504/capturerkv.png http://img263.imageshack.us/img263/7504/capturerkv.png

Is there something wrong with how my numbers are generated?

Answer 1

Jørn's answer gave the problem, but not the solution: just use a single instance of Random:

Random rng = new Random();

for (int i = 0; i < 50; i++)
{
    int lat = rng.Next(516400146, 630304598);
    int lon = rng.Next(224464416, 341194152);
    SamplePostData d0 = new SamplePostData();
    d0.Location = new Location(18d + lat / 1000000000d,
                               -72d - lon / 1000000000d);
    AddPushpin(d0);
}

(I've changed the way of using the result, too - there's no need to convert to and from a string to achieve what you want. An alternative would be to call Random.NextDouble instead and multiply it by the size of the desired range, then add a base value.)

If you're calling routine multiple times, you should probably use a single instance of Random across those multiple calls. Beware though: Random in .NET is not thread-safe. Ideally you should have a single instance per thread. See my article on randomness for more details of how to handle Random.

Answer 2

You are initializing both of your Random instances without an explicit seed, causing them to return the same stream of pseudo-random numbers. What you are doing now, is equivalent of using DateTime.Now as the seed for both instances. As modern computers tend to execute code really fast, you end up with the same seed in both instances.

If you use a single Random instance for generating both lang and long you should see a much more "random" distribution.