Shuffling a deck of cards

[vandel212]

I'm having an issue tyring to write a program that will shuffle multiple decks of cards like at a casino. I have assigned each card an ID in an array, so:
1 = 2 of dimonds
2 = 2 of hearts
3 = 2 of clubs
.
.
.
52 = Ace of spades

What I'm doing is generating a random number between 1 and 52 and filling an array with the random number which will be the card ID. I have it do a for each loop to search the array to see if the random number chosen is already in there. If it is, it breaks the loop and tries again.

There are no issues as far as errors are concerned, but it's incredibly inefficient. with 1 deck it doesn't long, but if there are 10 decks, the array size is 520 and it has to do the loop minimum 520 times. So it can take quite a bit of time to complete.

Is there a more efficient way of doing this?

Here is my code for shuffling:

private void ShuffleDeck()
        {
            CardOrder = new int[52];

            do
            {
                Random RandomNumber = new Random();
                int CardID = RandomNumber.Next(1, 53);
                bool UsedCard = true;

                foreach (int IDNumber in CardOrder)
                {
                    if (CardID == IDNumber)
                    {
                        UsedCard = true;
                        break;
                    }
                    else
                    {
                        UsedCard = false;
                    }
                }
                if (UsedCard == false)
                {
                    string cardcall = (CardListing[CardID - 1, 1]);
                    CardOrder[CardOrderIndex] = CardID;
                    listBox1.Items.Add((CardOrderIndex + 1).ToString() + ": " + CardListing[CardID - 1,1]);
                    CardOrderIndex++;
                }
            }
            while (CardOrderIndex < 52);
            CardOrderIndex = 0;
        }

[nelo]

There are no issues as far as errors are concerned, but it's incredibly inefficient. with 1 deck it doesn't long, but if there are 10 decks, the array size is 520 and it has to do the loop minimum 520 times. So it can take quite a bit of time to complete.

520 iterations shouldn't take a long time (off course it depends what you do in each iteration). Are you seeing any performance degradation as you add the number of decks?

[vandel212]

520 iterations shouldn't take a long time (off course it depends what you do in each iteration). Are you seeing any performance degradation as you add the number of decks?

Well it has to run through each element of the array 520 times. Being that there are 520 elements it it has to run through the loop a minimum of 520 * 520 which is 270400 times. With about 5 lines of code in each loop that comes out to 270400 * 5 which is 1.35 million lines of code... inefficient. That doesn't even include when it finds a repeated value and has to break and start the loop over.

I just can't think of another way to do it. Even if I made it a dynamically expanding array, I would still run into the issue once the array size started to get bigger. The only other thing I could think to do it get the cards on the fly which would speed it up much quicker. However I wanted to make it more like the real thing by having the deck order already built. So on the fly would make it less like the real deal (no pun intended).

Thanks.

[sotoasty]

Instead of using an array of numbers, use a generic list. Do a loop 51 times. Store values in a new list of cards. I haven't tried the code but something like this.

            System.Collections.Generic.List<Int32> cardList = new System.Collections.Generic.List<int>();
            System.Collections.Generic.List<Int32> newCardList = new System.Collections.Generic.List<int>();
            Random RandomNumber = new Random();
            // populate the original list
            for (int I = 1; I < 53; I ++)
            {
                cardList.Add(I);
            }
            // shuffle the deck
            for (int I = 0; I < 50; I ++)
            {
                int K = RandomNumber.Next(1, cardList.Count);

                newCardList.Add(cardList[K -1]);
                cardList.RemoveAt(K);
            }
            newCardList.Add(cardList[0]);

ETF: Changed newCardList.Add(cardList[K]);
TO
newCardList.Add(cardList[K -1]);

[BigEd781]

Well it has to run through each element of the array 520 times. Being that there are 520 elements it it has to run through the loop a minimum of 520 * 520 which is 270400 times. With about 5 lines of code in each loop that comes out to 270400 * 5 which is 1.35 million lines of code... inefficient.

Lines of code is not a measure of efficiency.

[MadHatter]

use Guids for your cards id's then simply sort (once) on the guid... nice and random, nice and quick. before each shuffle, re-id the card with a new guid.

[jonlist]

I'd make a Card class and a Deck class and enums for the suit, somthing like this:

    enum Suit { Heart, Diamond, Spades, Club };
    class Card
    {
        public Suit Suit { get; set; }
        public int Value { get; set; }
    }
    class Deck
    {
        private List<Card> inner;
        public int Count { get { return inner.Count; } }
        
        public Deck()
        {
            Reset();
            Shuffle();
        }

        public void Reset()
        {
            inner = new List<Card>();
            for (int suit = 0; suit < 4; suit++)
            {
                for (int value = 1; value < 15; value++)
                {
                    inner.Add(new Card() { Suit = (Suit)suit, Value = value });
                }
            }
        }

        public void Shuffle()
        {
            Random rnd=new Random();
            inner = inner.OrderBy(x => rnd.Next()).ToList();
        }

        public Card TakeTopCard()
        {
            var tmp = inner.First();
            inner.Remove(tmp);
            return tmp;
        }

        public Card PeekTopCard()
        {
            return inner.First();
        }

        public void AddCardToBottom(Card c)
        {
            inner.Add(c);
        }
    }

Sorry about the code spam, got a bit carried away