c# creating new instances of a object in a for loop

Question

I'm trying to create a new instance of a custom object inside a for loop, if i add a breakpoint i can see the object and properties changing and it returns x number of DIFFERENT candle objects. However, if i remove the breakpoint all the objects returned in the List are the same. Any ideas?

Thanks

    public List<candle> Getcandles(int can)
    {
        List<candle> dpl = new List<candle>();

        for (int i = 0; i < can; i++)
        {
            candle dp = new candle();
            dp.x = new Random().Next(0000, 9999);
            dp.y = new Random().Next(0000, 9999);              
            dpl.Add(dp);                

        }

        return dpl;

    }

score 8 · Accepted Answer · edited Apr 19 '11 at 14:07

8

You are not seeding your random generator. You should be sharing the same random instance across all calls to next:

var randomGenerator = new Random(DateTime.Now.Milliseconds);

Then, just call the one generator:

dp.x = randomGenerator.Next(0000, 9999);
dp.y = randomGenerator.Next(0000, 9999);

This way, you've both seeded the generator with something, and each call to next should generate a new 'random' number.

edited Apr 19 '11 at 14:07

Muad'Dib

28,542
5
55
68

answered Apr 19 '11 at 14:00

Tejs

40,736
10
68
86

2

Sam is seeding the RNG, but with the same seed (time), because the loop is so quick. So the same numbers are being produced for each instance of `candle` – Matt Ellen Apr 19 '11 at 14:02
Sam, using a breakpoint on the current code works fine because it adds a delay and changes the "seed" for your instance of Random. – BenCr Apr 19 '11 at 14:04

score 1 · Answer 2 · edited Apr 19 '11 at 14:04

System.Random(): From MSDN

Initializes a new instance of the Random class, using a time-dependent default seed value

without the debugger you are too fast.

try this:

public List<candle> Getcandles(int can)
{
    List<candle> dpl = new List<candle>();
    var rnd = new Random(DateTime.Now.Milliseconds);
    for (int i = 0; i < can; i++)
    {
        candle dp = new candle();
        dp.x = rnd.Next(0000, 9999);
        dp.y = rnd.Next(0000, 9999);              
        dpl.Add(dp);                

    }

    return dpl;

}

score 0 · Answer 3 · answered Apr 19 '11 at 14:02

Your instantiating a new Random() on each iteration. Because the loop is going so fast each Random() object is basically starting with the same value which is going to yield identical results.

Change your code to something like:

public List<candle> Getcandles(int can)     {
     List<candle> dpl = new List<candle>();
      Random generator = new Random();

      for (int i = 0; i < can; i++)         {
         candle dp = new candle();
         dp.x = generator.Next(0000, 9999);
         dp.y = generator.Next(0000, 9999);
         dpl.Add(dp);
       }
      return dpl;
  }

score 0 · Answer 4 · answered Apr 19 '11 at 14:04

Take the new Random() outside the for loop.

    public List<candle> Getcandles(int can)
    {
        List<candle> dpl = new List<candle>();
        var random =new Random() 

        for (int i = 0; i < can; i++)
        {
            candle dp = new candle();
            dp.x = random .Next(0000, 9999);
            dp.y = random .Next(0000, 9999);              
            dpl.Add(dp);                

        }

        return dpl;

    }

c# creating new instances of a object in a for loop

4 Answers4