-1

One function inputs 24 random strings into an string list from an array with a for loop. The other function displays the strings from the list in the console. However I get only one value looped for example like 1111111111111111111111111. When I set breakpoint, I get the output I was looking for such as 12kingace342356110. Here's my function class code.

namespace CardWarConsoleGame
{
    class Deck
    {
        public string[] CardNames = { "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "Jack", "Queen", "King", "Ace" };
        public int[] CardValues = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14 };
    }


    class WarFunctions
    {
        public static List<string> YourDeck = new List<string>();
        public static List<string> AIDeck = new List<string>();

        public static void LoadCards()
        {
            Deck deck = new Deck();
            for (int i = 0; i < 24; i++)
            {
                Random r = new Random();
                YourDeck.Add(deck.CardNames[r.Next(0, 14)]);
            }

        }

        public static void test()
        {
            for (int i = 0; i < YourDeck.Count; i++)
            {
                Console.Write(YourDeck[i]);
            }
        }
    }
}



Heres the program.cs

namespace CardWarConsoleGame
{
    class Program : WarFunctions
    {
        static void Main(string[] args)
        {
            LoadCards();
            test();
            Console.ReadLine();
        }
    }
}
MethodMan
  • 18,625
  • 6
  • 34
  • 52
Lions L
  • 11
  • 3

3 Answers3

4

You should create the Random before the loop.

public static void LoadCards()
{
    Deck deck = new Deck();
    Random r = new Random();
    for (int i = 0; i < 24; i++)
    {
        YourDeck.Add(deck.CardNames[r.Next(0, 14)]);
    }
}

The default constructor of Random

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

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.

Community
  • 1
  • 1
Jakub Lortz
  • 14,616
  • 3
  • 25
  • 39
1
 public static void LoadCards()
 {
     Deck deck = new Deck();
     for (int i = 0; i < 24; i++)
     {
         Random r = new Random();
         YourDeck.Add(deck.CardNames[r.Next(0, 14)]);
     }
  }

Your Random class should not be created in the foreach loop. If you keep initalizing random within the foreach loop when it seeds itself to produce random number you keep starting that sequence over based on time of system clock. If you place it outside the foreach loop it will maintain it's initial seed data and produce numbers without reseeding back to the first one.

So your code would look like this:

 public static void LoadCards()
 {
     //Random initialized outside of the foreach loop
     Random r = new Random();
     Deck deck = new Deck();
     for (int i = 0; i < 24; i++)
     {
         YourDeck.Add(deck.CardNames[r.Next(0, 14)]);
     }
  }

Documentation on default constructor for Random.

Cubicle.Jockey
  • 3,288
  • 1
  • 19
  • 31
1

Check this link. They describe it beautifully The problem is that you are creating instances of the Random class too close in time.

When you create a Random object, it's seeded with a value from the system clock. If you create Random instances too close in time, they will all be seeded with the same random sequence.

Create a single Random object and pass its reference to the constructor when you create instances of the "a" class, instead of creating one Random object for each "a" instance.

Community
  • 1
  • 1
Anik Saha
  • 4,313
  • 2
  • 26
  • 41