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;

    }

Muad'Dib · Accepted Answer · 2011-04-19 14:07:52Z

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, 2011 at 14:07

Muad'Dib

29.4k5 gold badges58 silver badges71 bronze badges

answered Apr 19, 2011 at 14:00

Tejs

41.3k10 gold badges71 silver badges89 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Matt Ellen Over a year ago

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

BenCr Over a year ago

Sam, using a breakpoint on the current code works fine because it adds a delay and changes the "seed" for your instance of Random.

ChrisLively · Accepted Answer · 2011-04-19 14:04:30Z

1

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;

}

edited Apr 19, 2011 at 14:04

ChrisLively

88.2k27 gold badges174 silver badges249 bronze badges

answered Apr 19, 2011 at 14:02

Arthur

8,1493 gold badges32 silver badges46 bronze badges

Comments

ChrisLively · Accepted Answer · 2011-04-19 14:02:30Z

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;
  }

Nasmi Sabeer · Accepted Answer · 2011-04-19 14:04:03Z

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;

    }

Collectives™ on Stack Overflow

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

4 Answers 4

2 Comments

Comments

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

Comments

Comments

Comments

Related