0

I'm a beginner programmer and need some help. In the beginning of my program, 5 bunnies (object type Bunny) are created. It looks like this:

List<Bunny> bunnies = new List<Bunny>();
Bunny newBunny;
for(int i = 0; i < 5; i++)
{
    newBunny = new Bunny();
    bunnies.Add(newBunny);
 }

My problem is that the construction of newBunny takes too long and the program continues with the for-loop. This causes the list to have just the first constructed newBunny and duplicates of it.Running the code prints me the following:

Name-Kate, Gender-Male, Color-White, Age-0, Radioactive-False

Name-Kate, Gender-Male, Color-White, Age-0, Radioactive-False

Name-Kate, Gender-Male, Color-White, Age-0, Radioactive-False

Name-Kate, Gender-Male, Color-White, Age-0, Radioactive-False

Name-Kate, Gender-Male, Color-White, Age-0, Radioactive-False

The construction looks like this:

public Bunny()
{
     Random randNum = new Random ();
     int namesCount = Enum.GetNames(typeof(BunnyName)).Length;
     Name=((BunnyName)(randNum. Next(0, namesCount)));

     int genderCount = Enum.GetNames(typeof(BunnyGender)).Length;
     Gender=((BunnyGender)(randNum. Next(0, genderCount)));

     int colorCount = Enum.GetNames(typeof(BunnyColor)).Length;
     Color=((BunnyColor)(randNum. Next(0, colorCount)));

     Age=-1;

     if(randNum. Next(1,101)<2)
         Radioactive = true;
     else
         Radioactive =false;
} 

I want the program to halt until the construction of newBunny ends, and only then to add it to the list:

List<Bunny> bunnies = new List<Bunny>();
Bunny newBunny;
for(int i = 0; i < 5; i++)
{
    //stop until next line finishes
    newBunny = new Bunny();
    //continue
    bunnies.Add(newBunny);
 }

Ps. If I'm debugging the program it runs just fine, exactly like i want it to (new bunnies not 'duplicates') Also, if I add a messageBox in the end of the for-loop, everything works good. This is why i thought it might need a halt

for(int i = 0; i < 5; i++)
{
    //stop until next line finishes
    newBunny = new Bunny();
    //continue
    bunnies.Add(newBunny);
    MessageBox.Show("test");
 }

Hope you understood my problem, thanks.

Shachar Levavi
  • 95
  • 1
  • 1
  • 5
  • 11
    "My problem is that the construction of newBunny takes too long and the program continues with the for-loop. This causes the list to have just the first constructed newBunny and duplicates of it." No, that's just not true. It's not clear why you think that's happening, but it's not. Please provide a short but complete program demonstrating the problem, and we can help you diagnose it. Also note that your code would be better (IMO) by declaring the variable *inside* the loop instead of outside - in general, use the smallest possible scope for local variables. – Jon Skeet Feb 08 '15 at 19:48
  • And how did you determine it's the same object? – Tarik Feb 08 '15 at 19:55
  • This behaviour could be caused if using non-blocking threads which are launched inside the constructor (used to initialise your object's properties), but seeing as you're a new programmer it is unlikely that you are doing this. – Aalawlx Feb 08 '15 at 19:57
  • I don't think the OP is starting threads at this point. As he pointed out he is a beginner. – Philip Stuyck Feb 08 '15 at 20:01
  • @GrantWinney yes but it might *appear* as though it isn't a new instance as the properties are not initialised - just like the previous object. – Aalawlx Feb 08 '15 at 20:02
  • Show us the constructor of Bunny. I don't think it is taking too long at all. What makes you think so ? – Philip Stuyck Feb 08 '15 at 20:05
  • @PhilipStuyck it will take me some time to write it, because I'm using my phone. The constructor randoms between enums – Shachar Levavi Feb 08 '15 at 20:08
  • 1
    All of your `Random`s have the same seed. – SLaks Feb 08 '15 at 20:32
  • 1
    do this : static Random randNum = new Random (); Your program is actually very fast. The Random is using the same time based seed, and generates always the same numbers, by making it static, there will be only one Random instance created and reused all the time generating new random numbers instead of always the same ones. – Philip Stuyck Feb 08 '15 at 20:36

3 Answers3

1

Your program execution is totally synchronous so in fact there are 5 different instances of Bunny in the list. If you don't believe me change the constructor of Bunny and add an integer to it. In the constructor assign this integer to an instance variable of Bunny like so:

public class Bunny{
    private int _instanceId;

    public Bunny(int instanceId){
        _instanceId = instanceId;
    }
}

in the loop do this :

newBunny = new Bunny(i);

now use the debugger to step through the code. Hopefully visual studio. Put a break point on the add statement and navigate/hover the cursor over the bunnies variable to see what is inside. You can even inspect the different instances and you will see the instanceid is different. Your code just creates 5 different instances of Bunny, but because the constructor is the same, you end up with 5 instances that look exactly the same.

To get really random values do this in your constructor :

static Random randNum = new Random ();
Philip Stuyck
  • 7,344
  • 3
  • 28
  • 39
1

Random in a loop? They may be different objects but all your random numbers are the same.

This is because the constructor for Random, when given no parameters uses a seed from the system clock. So if you new them up quickly like this they all end up the same and will give the same value when you call Next.

Much better to pass one Random into all of the constructors and use that instead.

List<Bunny> bunnies = new List<Bunny>();
Random random = new Random();
for(int i = 0; i < 5; i++)
{
    bunnies.Add(new Bunny(random));
}

You also don't need a variable to hold the bunnies at an unnecessarily high scope. If your using something in the for loop only, it doesn't need to exist outside of it.

Having a static Random in the Bunny class has been suggested, but i'd advise against it. There are a lot of benefits of injecting things into the instance (especially when mult-threading), but in your case the advantage is testability.

I've included a class below as an example of something you could use to test your Bunny constructor, you can control the bunny that gets made and then verify that what happened is correct:

class MyRandomIsAlwaysN : Random
 {
    private readonly int nextValue;

    public MyRandomIsAlwaysN(int n){
        this.nextValue = n;
    }
    public override int Next(int x, int y){
         return this.nextValue
    }
 }
Community
  • 1
  • 1
Nathan Cooper
  • 6,262
  • 4
  • 36
  • 75
1

The problem you have is in your usage of Random.

See here: C# Random Numbers aren't being "random"

I would suggest you create one Random class outside of bunny, and then pass that into the constructor.

i.e

List<Bunny> bunnies = new List<Bunny>();
Bunny newBunny;
Random randomGenerator = new Random();
for(int i = 0; i < 5; i++)
{
    newBunny = new Bunny(randomGenerator);
    bunnies.Add(newBunny);
}
Community
  • 1
  • 1
hatcyl
  • 2,190
  • 2
  • 21
  • 24