Why does my card code show duplicate cards?

Question

I'm creating a program for deck of cards. It works fine but only error I get is that it shows duplicate cards - no card should be displayed twice. Tried all you guys suggested. basically this is brand new for me and all the advance methodology you are talking about haven't been taught yet. Also searched and looked for all the solutions available for deck of cards, doesn't help though.

namespace DeckofCards
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.Title = "DECKOFCARDS";
            String UI = "";
            while (UI.ToUpper() != "Q")
            {
                Console.Clear();

                Console.WriteLine("Please Press Q to Quit");
                UI = Console.ReadLine();
                if (UI.ToUpper() == "Q")
                {
                    break;
                }

                List<String> Faces = new List<string>();
                List<String> Suits = new List<string>();

                Faces.Add("Ace");
                Faces.Add("1");
                Faces.Add("2");
                Faces.Add("3");
                Faces.Add("4");
                Faces.Add("5");
                Faces.Add("6");
                Faces.Add("7");
                Faces.Add("8");
                Faces.Add("9");
                Faces.Add("10");
                Faces.Add("Jack");
                Faces.Add("Queen");
                Faces.Add("King");

                Suits.Add("Hearts");
                Suits.Add("Spades");
                Suits.Add("Clubs");
                Suits.Add("Diamonds");

                Console.WriteLine("How many cards do you want.?");
                string Input = Console.ReadLine();
                int NumberOfCards = 0;
                Random RanFaces = new Random();
                Random RanSuits = new Random();

                {
                    if (int.TryParse(Input, out (NumberOfCards))) // to handle incorrect (alphabetic) inputs

                        if (NumberOfCards <= 52 && NumberOfCards > 0)
                        {
                            for (int Output = 0; Output < NumberOfCards; ++Output)
                            {
                                int RandomFaces = RanFaces.Next(0, 12);
                                int RandomSuits = RanSuits.Next(0, 3);

                                Console.WriteLine("Your Cards are {0} of {1}", Faces[RandomFaces], Suits[RandomSuits]);
                                if ((Output + 1) % 4 == 0)
                                    Console.WriteLine();
                            }
                        }
                        else
                        {
                            Console.WriteLine("This is not a Valid number of cards.");
                        }
                    else
                    {
                        Console.WriteLine("This is not a Valid number of cards.");
                    }
                    Console.ReadLine();
                }
            }
        }
    }
}

You don't want to pick a random suit or face; that allows the same card to be picked multiple times. What you want is called a [shuffle](https://stackoverflow.com/questions/10688044/). — Dour High Arch, Oct 09 '18 at 16:54
You algorithm will not work. You are creating randomize cards which creates duplicates. You have to create a deck of 52 cards and assign a random number to each card. Then sort the deck by the random number. — jdweng, Oct 09 '18 at 16:55
Hello. I see no attempt in your code to avoid duplicate cards, like storing which cards have already bean dealt for example. Also note that "Ace" and "1" are actually the same so there's no sense having both... — Laurent S., Oct 09 '18 at 16:55

score 1 · Answer 1 · answered Oct 09 '18 at 16:55

1

Try to think in real-world terms and tailor your algorithm accordingly. To get a shuffled deck, you first must first have a deck containing every card. Then, you'll apply the technique to randomize the ordering of the cards. It looks like your current approach is trying to create your deck through generating random cards instead.

answered Oct 09 '18 at 16:55

Jacob

77,566
24
149
228

1

The main problem here being that this is not a random number generator per se - you can not draw the exact same card twice from a deck, and the same number onl 4 times (in color variants). – TomTom Oct 09 '18 at 16:57

RBarryYoung · Answer 2 · 2018-10-09T17:06:13.050

1

The standard way to do this is to first create a "Deck" of all 52 possible cards occurring once each in order. The "Deck" is just some ordered structure like an array.

Then you "shuffle" that deck by going through each position in the deck and swapping the card at that position with another card from a random position in the deck (possibly itself). This then assures that the cards are in a random order and every card occurs once and only once.

Its not well known but this naive approach has a bug in it: if you randomly swap each card with any other position, the resulting distribution of cards is not perfectly random. The fix to this is to swap each card position in turn only with card positions equal to or greater than its current position. (This is called a Fisher-Yates Shuffle)

edited Oct 09 '18 at 17:06

answered Oct 09 '18 at 17:00

RBarryYoung

55,398
14
96
137

Seems to me like a simple `OrderBy(_ => random.Next()).ToList()` should be a sufficient "shuffling" algorithm. A 52-card deck is hardly going to take enough memory to require inline shuffling. Fisher-Yates is an interesting bit of trivia, though. – StriplingWarrior Oct 09 '18 at 17:11
@StriplingWarrior Sorting is *not* simpler and certainly not faster. Since this is for a school class, I'm assuming that they haven't learned LINQ and Lambdas in C# yet, and may not even touch on it. The algorithm to implement this is very simple, as well as instructive. I have left it as an exercise for the OP, because this is after all homework. – RBarryYoung Oct 09 '18 at 17:17
Yeah, I certainly wouldn't claim it's faster: it definitely requires more memory and CPU. But as a practical matter, simpler code trumps tiny performance gains. You do make good points about the value of this answer from a scholastic standpoint, though. – StriplingWarrior Oct 09 '18 at 22:46

score 0 · Answer 3 · answered Oct 09 '18 at 16:56

A classic school problem.

A good technique is to create a custom class that contains the card description and a floating point number. Build a list of 52 objects of that class, one for each card. To shuffle the deck: Assign a random number to the floating point value in each object. Sort the list by the floating point number. This gets a randomly ordered list where there are no repeats.

Why does my card code show duplicate cards?

3 Answers3