How to shuffle a List<T>

Question

I found this piece of Java code at Wikipedia that is supposed to shuffle an array in place:

public static void shuffle (int[] array)
{
    Random rng = new Random();
    int n = array.length;
    while (n > 1) 
    {
        n--;  
        int k = rng.nextInt(n + 1);
        int tmp = array[k];
        array[k] = array[n];
        array[n] = tmp;
    } 
}

Though I didn't test the code, it looks like it should work just fine with an array. In my C# project I created a CardSet class and used the above code in a Shuffle() method:

public class CardSet
{
    private List<Card> cards;

    public Card this[int i]
    {
        get {  return cards[i];  }
        set {  cards[i] = value;  }
    }

    public void Shuffle()
    {
        Random rng = new Random();
        int n = this.NumberOfCards;

        while (n < 1)
        {
            n--;
            int k = rng.Next(n + 1);
            Card tmp = this[k];
            this[k] = this[n];
            this[n] = tmp;
        }
    }

When I use the method, however, no shuffling happens:

CardSet cs = new CardSet();
cs.Shuffle(); 

foreach (Card c in cs)
{
    Console.WriteLine(c.ToString());
}

I just can't figure out why it doesn't work. I thought that the List might be automatically sorting its objects, so I tried changing one of its values,

cs[8] = new Card(Suites.Hearts, Numbers.Two);

and that Card was exactly where I put it. Either I made some simple mistake or I didn't write the shuffling algorithm correctly. If the code I supplied looks good and someone thinks the mistake might be somewhere else in my code, I can supply the rest of my code.

Answer 1

look into LINQ and orderby with a GUID

Answer 2

Change

while (n < 1)

to

while (n > 1)

Answer 3

Your while loop is off. It says while n is less than 1. You set n to the number of cards. So say if you have 52 cards n is definitely greater than 1 and your loop doesn't execute.

so change your while loop to look like this:

while(n > 1)

Answer 4

The code in your example method and the code in your application (the first and second blocks) are different. In one, you use

While (n < 1)

And in the other you use

While (n > 1)

The correct one to use is in your sample - the "(n > 1)". If you use the other one, your loop doesn't execute even a single time - it skips over the condition and your deck is left untouched.

That said, LINQ is a better option if it's available to you.

Answer 5

This has been asked before - see Randomize a List in C# on SO.