Shuffling a deck of cards with images

Question

Im trying to randomize a list and then put it out to an picturebox. When i do this i get the error message "Index was out of range. Must be non-negative and less then the size of the collection"

Here is the code for random:

public class Dealer
{
    public static Random rand = new Random();
    public static List<Kort> KortenÄrBlandade = new List<Kort>();
    public void Shuffle()
    {
        List<Kort> KortenÄrBlandade = new List<Kort>(deckOfCards.OrderBy(_ => rand.Next(0, deckOfCards.Count)));
    }
}

And here is the code for calling the method

private void button1_Click(object sender, EventArgs e)
{
    Spelare.Dealer deal = new Spelare.Dealer();
    deal.Shuffle();
    pictureBox1.Image = Spelare.Dealer.KortenÄrBlandade[2].img;
}

Answer 1

You can try using Fisher-Yates shuffle algorithm

http://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle

It could be something like that:

public class Dealer {

  // Fisher-Yates shuffle algorithm with explicit generator
  private static void CoreShuffle(IList<Kort> list, Random generator) {
    if (Object.ReferenceEquals(null, list))
      throw new ArgumentNullException("list");
    else if (Object.ReferenceEquals(null, generator))
      throw new ArgumentNullException("generator");

    for (int i = list.Count - 1; i >= 0; --i) {
      int index = generator.Next(i + 1);

      Kurt h = list[index];
      list[index] = list[i];
      list[i] = h;
    }  
  }

  public static Random rand = new Random(); // <- Be careful: Random is not thread safe; readonly is also a good addition here

  // Return new shuffled list based on deckOfCards      
  public List<Kort> ShuffledList() {
    List<Kort> result = new List<Kort>(deckOfCards); // <- Check this line in your code
    CoreShuffle(result, rand);

    return result;   
  }

Answer 2

Your Shuffle method doesn't shuffle the static list, but creates (and forgets) a variable that is local to the method. So your Spelare.Dealer.KortenÄrBlandade is never set to any real cardlist. Which means that index 2 doesn't exist, which is what the error message says.

A side remark: your mix of a static cardlist + instance Shuffle method is a bit strange ...

Answer 3

public void Shuffle()
{
    List<Kort> KortenÄrBlandade = new List<Kort>(...);
}

The Shuffle method uses a local variable that has the same name as Dealer.KortenÄrBlandade. So after the method returns, you still have an empty list. Fix:

public void Shuffle()
{
    Dealer.KortenÄrBlandade = new List<Kort>(...);
}

Do note that this is not a proper shuffle. Class design is not correct either, either Shuffle() should be static or KortenÄrBlandade should not be static. Confusion about the storage of variables is how you ended up writing this bug.