0

I have a list private List<HeroStats> allHeroes;

and a list private List<Team> allTeams;

After I populate my list 'allHeroes' with heroes , I want to create teams of 5 random heroes, using this method:

public Team createTeam()
{
    int index=0;
    Team t = new Team();
    Random rnd = new Random();
    int cap = allHeroes.Count;
    while (t.count() < 5)
    {
        index = rnd.Next(0, cap);
        t.Add(allHeroes.ElementAt(index));
    }
    return t;
}


This creates a perfect team , but if i want to create more teams, it will generate the same team over and over again.
I also have a method

public List<HeroStats> print()
{
    StringBuilder sb = new StringBuilder();
    List<HeroStats> l = new List<HeroStats>();
    foreach (HeroStats h in team)
    {
         sb.AppendLine(h.HeroName);
         l.Add(h);
    }
    Console.WriteLine(sb.ToString());
    return l; 
}

Which should print out the name of the heroes in a team.

Why am I getting the same team if I generate many ?



To create multiple teams I use :

Team a = new Team();
            for (int i = 0; i < 2000; i++)
            {
                a = createTeam();
               allTeams.Add(a);

         }
Bogdan Molinger
  • 251
  • 1
  • 6
  • 19

5 Answers5

2

It is creating the same teams because the Random instance is created in the method which is causing the same seed if you call createTeam very fast (the default Random constructor uses the current time as seed). You can avoid that either by passing the Random to the method or by using a field:

public Team createTeam(Random rnd) // as an aside, you should call it CreateRandomTeam
{
    int index=0;
    Team t = new Team();
    int cap = allHeroes.Count;
    while (t.count() < 5)
    {
        index = rnd.Next(0, cap);
        t.Add(allHeroes.ElementAt(index));
    }
    return t;
}

Now you have to ensure that you pass always the same Random instance to createTeam.

For example with your loop:

Random rnd = new Random();
Team a = new Team();
for (int i = 0; i < 2000; i++)
{
    a = createTeam(rnd);
    allTeams.Add(a);
}

MSDN also mentions this in the remarks section:

The default seed value is derived from the system clock and has finite resolution. As a result, different Random objects that are created in close succession by a call to the default constructor will have identical default seed values and, therefore, will produce identical sets of random numbers. This problem can be avoided by using a single Random object to generate all random numbers. You can also work around it by modifying the seed value returned by the system clock and then explicitly providing this new seed value to the Random(Int32) constructor

Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
1

You need to have

 Random rnd = new Random();

Outside of

public Team createTeam()
{

}

Maybe parse in the rnd as a parameter.

Catwood
  • 157
  • 7
1

From the documentation of Random

The default seed value is derived from the system clock and has finite resolution. As a result, different Random objects that are created in close succession by a call to the default constructor will have identical default seed values and, therefore, will produce identical sets of random numbers.

If you are calling createTeam() in a tight loop to create your random teams, you will end up with the same set of random numbers due to the fact that your instances of Random are create so close to each other and with the same seed.

A possible solution is to take out the object of type Random and make it a class level field.

public static class RandomTeamCreator
{
    private static readonly Random _random = new Random();
    public Team CreateTeam()
    {
        // create team using _random
    }
}
Tejas Sharma
  • 3,420
  • 22
  • 35
1

Why am I getting the same team if I generate many ?

Probably because the time between creating teams is not enough for the internal clock to create a new random seed.

I would add a "seed" parameter to createTeam:

public Team createTeam(int seed)
{
    int index=0;
    Team t = new Team();
    Random rnd = new Random(seed);
    int cap = allHeroes.Count;
    while (t.count() < 5)
    {

        index = rnd.Next(0, cap);
        t.Add(allHeroes.ElementAt(index));
    }
    return t;
}

And then use another Random outside of th eloop to generate the seed:

for(int 1 = 0; i < 10; i++)
{
   Random r = new Random(0);
   int seed = r.Next();
   createTeam(int seed);
}

If you wanted to keep the original signature just add an overload:

public Team createTeam()
{
    return createTeam(new Random().Next());
}

SIDE NOTE

You _probably want a "shuffled" team of heroes rather than a "random" since with "random" you could get the same hero twice. If that is the case then just use an "order by" with a random order:

public Team createTeam(int seed)
{
    Random rnd = new Random(seed);

    Team t = new Team();

    var shuffled = allHeroes.OrderBy(rnd.Next()).Take(5);
    foreach(var hero in shuffled)
        t.Add(hero);

    return t;
}
D Stanley
  • 149,601
  • 11
  • 178
  • 240
0

If you create many Random instances at roughly the same time, they are likely to generate the same numbers. Instead, create one Random somewhere else and call its Next method from your algorithm.

Theodoros Chatzigiannakis
  • 28,773
  • 8
  • 68
  • 104