ArgumentsOutOfRangeExeption in a list

Question

Im getting this exeption thrown when the method is invoked. the list contains exactly 52 objects(number of cards). Any suggestions what may cause it? maybe the Add and RemoveAt Methods? Or maybe the Random? The compiler also tell the the problem is in deck.Add(temp[j]); line.

public void Shuffle()
        {
            List<Card> temp = new List<Card>();
            Random rand = new Random();
            for (int i = 0; i < 52; i++)
            {
                for (int j = rand.Next(i, 52); j < 52; j++)
                {
                    temp.Add(deck[j]);
                    deck.RemoveAt(j);
                    deck.Add(temp[j]);
                }
            }
       }

this code.. What is the point of that inner loop if you're not going to use it to loop? — Jeroen Vannevel, Jan 04 '15 at 17:10
Its just a demo for the shuffle method , nothing special right now. — God, Jan 04 '15 at 17:18
I think your real problem is, that you call `deck.RemoveAt(j);` before `deck.Add(temp[j]);`. So you may be trying to delete an entry at e.g. index 42 that's not yet there. — Jan Köhler, Jan 04 '15 at 17:32

score 4 · Accepted Answer · answered Jan 04 '15 at 17:39

Ok, let's imagine we are on the first run through the loops. First iteration of both the outer and inner loop. i is 0, and Rand(i, 52) produces 13.

So we have:

i - 0
j - 13
temp - empty list
deck - assume this is a list with 52 elements

Now let's run the three lines of code inside the loop:

temp.Add(deck[j]);

Get the 13th item of deck and add it to temp. Ok, done. temp now has 1 item.

 deck.RemoveAt(j);

Remove the 13th item in deck. Ok, fine.

 deck.Add(temp[j]);

Get the 13th item in temp and add it to, wait, what?1? temp only has 1 item! Exception! Exception!.

There isn't a need for the temp list in the first place. There is a very good shuffling algorithm that involves going through the original list of items once (not N times). And you just need one temp variable to hold a value while you swap it with another. Like this:

public void Shuffle()
{
    Card temp;
    Random rand = new Random();
    for (int i = deck.Length; i >= 1; i--)
    {
        int j = rand.Next(0, i + 1);
        temp = deck[i];
        deck[i] = deck[j];
        deck[j] = temp;
   }
}

Done.

Yeah, your shuffle code really works, thanks all. although im gonna stay with mine untill it will work. I dont like copy pasting codes. — God, Jan 04 '15 at 18:32
@God Fair enough. If you can describe in words how you envision your approach working, perhaps somebody could help you to get it to work. I know you gave a short description to omikad, but if you could tell us in more detail what _process_ you are trying to use (e.g. why two loops? why is `temp` a list?), we might be able to point you in the right direction. — JLRishe, Jan 04 '15 at 19:57
Thanks. I still i wanted it to do outerLoop * innerLoop number of shuffles. That why i used the outer loop. and for the temp list i guess that was a bad idea and a temp object is better. Thanks again! — God, Jan 05 '15 at 15:34

score 2 · Answer 2 · answered Jan 04 '15 at 17:31

2

If you want to shuffle list of items then you can use the following method:

    public static void Shuffle<T>(IList<T> arr, Random rnd)
    {
        for (var i = 0; i < arr.Count; i++)
        {
            var j = rnd.Next(i, arr.Count);

            var tmp = arr[i];
            arr[i] = arr[j];
            arr[j] = tmp;
        }
    }

This method will help you shuffle your deck without ArgumentsOutOfRangeExeption

answered Jan 04 '15 at 17:31

omikad

979
8
10

I will try it eventually if i will not find a solution. but there a method that shuffles the deck with my code idea?? thanks you all. – God Jan 04 '15 at 17:35
@God Your code idea makes no sense and even if it weren't throwing exceptions, it's extremely inefficient. So it's unlikely there's a shuffling method that uses it. – JLRishe Jan 04 '15 at 17:40
@God please write your idea by words. Do you want to create some interesting probability distribution? All answers here corresponds to the uniform distribution. – omikad Jan 04 '15 at 17:46
Shuflle the deck 52 times each time with a random number between 0 - 51 – God Jan 04 '15 at 18:24

score 1 · Answer 3 · answered Jan 04 '15 at 17:08

1

temp[j] does not neccessarily exist. You will need to initialize temp so it has at least j+1 entries or you need to change the line of temp[j] to something more fitting.

answered Jan 04 '15 at 17:08

nvoigt

75,013
26
93
142

I initilized the list so now it exist. Still throws the Exeption. And what could be more fitting in your though? – God Jan 04 '15 at 17:22

score 1 · Answer 4 · answered Jan 04 '15 at 17:08

1

When you call rand.Next(i, 52), the result could be 52, which would be out of range for your deck and temporary deck.

Also, you need to initialize your temp list, as @nvoigt points out. List has a constructor that takes an integer for the initial size. You could pass in 52. See: http://msdn.microsoft.com/en-us/library/dw8e0z9z(v=vs.110).aspx.

You could have also easily debugged this yourself by looking at the value of j in your debugger.

answered Jan 04 '15 at 17:08

siride

200,666
4
41
62

I did all you said(Initilize the list to 52 objects and rand.Next(1,51)) and its still throws that eception.. – God Jan 04 '15 at 17:19
@siride: No, the maximum can't be 52 since the second argument of `Next` is the **exclusive** upper bound: http://msdn.microsoft.com/en-us/library/2dx6wyd4%28v=vs.110%29.aspx – Jan Köhler Jan 04 '15 at 17:30
So i get your head i need to initilize it to 51?? – God Jan 04 '15 at 17:33
Got ya. but it still Throw the Exeption. – God Jan 04 '15 at 17:37
Sorry, misunderstood the docs. Looks like there's a better answer anyway. – siride Jan 05 '15 at 02:50

ArgumentsOutOfRangeExeption in a list

4 Answers4