Index out of range.... but it doesn't appear to be out of range so whats going on?

Question

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace CardDealer
{
    class Deck
    {

        string[] deck = new string[52];

        string[] name = new string[13];

        string[] suits = new string[4];

        public Deck()
        {
            name[0] = "Ace ";
            name[1] = "Two ";
            name[2] = "Three ";
            name[3] = "Four ";
            name[4] = "Five ";
            name[5] = "Six ";
            name[6] = "Seven ";
            name[7] = "Eight ";
            name[8] = "Nine ";
            name[9] = "Ten ";
            name[10] = "Unterknave ";
            name[11] = "Oberknave ";
            name[12] = "King ";

            suits[0] = "of Hearts";
            suits[1] = "of Bells";
            suits[2] = "of Acorns";
            suits[4] = "of Leaves";

            for (int i = 0; i < 13; i++)
            {
                deck[i] = name[i] + suits[0];
            }

            for (int i = 0; i < 13; i++)
            {
                deck[i + 13] = name[i] + suits[1];
            }

            for (int i = 0; i < 13; i++)
            {
                deck[i + 26] = name[i] + suits[2];
            }

            for (int i = 0; i < 13; i++)
            {
                deck[i + 39] = name[i] + suits[3];
            }
        }

        Random rnd = new Random();
        int cardsLeft = 52;

        public void Shuffle()
        {
            string[] deck = new string[52];

            for (int i = 0; i < 13; i++)
            {
                deck[i] = name[i] + suits[0];
            }

            for (int i = 0; i < 13; i++)
            {
                deck[i + 13] = name[i] + suits[1];
            }

            for (int i = 0; i < 13; i++)
            {
                deck[i + 26] = name[i] + suits[2];
            }

            for (int i = 0; i < 13; i++)
            {
                deck[i + 39] = name[i] + suits[3];
            }


            string[] myrandomarray = deck.OrderBy(x => rnd.Next()).ToArray();

            deck = myrandomarray;

            cardsLeft = 52;
        }



        public string Deal()
        {

            string deltCard = deck[0];

            cardsLeft--;

            string[] newDeck = new string[cardsLeft];

            for (int i = 0; i < cardsLeft + 1; i++)
            {
                if (deck[i] != deltCard)
                {
                    newDeck[0] = deck[i];
                }
            }

            deck = newDeck;

            return deltCard;





        }


    }

    class play
    {
        static void Main()
        {

            Deck mydeck = new Deck();

            Console.WriteLine(mydeck.Deal());


        }
    }



}

Hey guys I'm having trouble here with my code. There are two classes, one constructs a deck, and the other plays it. when I execute this code, it throws an error (range out of index) But I'm just not seeing it, like I don't know how is it out of range, it's showing me the place where it's out of range but it doesn't seem to be out of range so whats going on?

To the best of my knowledge, this exception is only thrown a) when the index is out of range, or b) when you're misusing some form of collection intended for single threaded use from multiple threads. Your instinct *shouldn't* be to assume that the exception is thrown incorrectly. — Damien_The_Unbeliever, Oct 16 '18 at 12:31
i don't think that sof is made for helping people debug code... — Haithem KAROUI, Oct 16 '18 at 12:33
You should have a look at [this question](https://stackoverflow.com/questions/1150646/card-shuffling-in-c-sharp) for another approach. — Guy, Oct 16 '18 at 12:42
Side note: If you want *shuffle* the deck, `deck.OrderBy(x => rnd.Next())` is not a good choice; see https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle — Dmitry Bychenko, Oct 16 '18 at 12:43
This question was caused by a problem that can no longer be reproduced or a simple typographical error. While similar questions may be on-topic here, this one was resolved in a manner unlikely to help future readers. This can often be avoided by identifying and closely inspecting [the shortest program necessary to reproduce the problem](https://stackoverflow.com/help/mcve) before posting. — Broots Waymb, Oct 16 '18 at 12:46
Side note, problems like this are perfect cases in which to step through the code via debugger. It would have brought you right to the troublesome line of code (debug stack trace would have helped a lot as well). — Broots Waymb, Oct 16 '18 at 12:51

score 3 · Answer 1 · answered Oct 16 '18 at 12:28

3

You are missing index 3, instead you have 4:

suits[0] = "of Hearts";
suits[1] = "of Bells";
suits[2] = "of Acorns";
suits[4] = "of Leaves";

Change last line to:

suits[3] = "of Leaves";

answered Oct 16 '18 at 12:28

Ashkan Mobayen Khiabani

33,575
33
102
171

looks we are having a race to answer! :-) – jazb Oct 16 '18 at 12:30
Sorry, that's a stupid error on my part... oops. – John Oct 16 '18 at 12:30
2

@John, if this solved your problem, please mark it as accepted. – Viggo Lundén Oct 16 '18 at 12:31
@John this happens to everyone. don't blame yourself. – Ashkan Mobayen Khiabani Oct 16 '18 at 12:31

Dmitry Bychenko · Answer 2 · 2018-10-16T12:49:37.493

Do not put indexes explicitly:

        string[] suits = new string[4];

        ...

        suits[0] = "of Hearts";
        suits[1] = "of Bells";
        suits[2] = "of Acorns";
        suits[4] = "of Leaves";  // <- it must be "3", not "4"

But use initializer:

        // create array with items mentioned
        string[] suits = new string[] {
          "of Hearts", 
          "of Bells", 
          "of Acorns", 
          "of Leaves"
        };

Let the compiler do the routine work for you. The same with name and deck:

// Let's get rid of trailing spaces
string[] name = new string[] {
  "Ace", 
  "Two",
  "Three",
  "Four",
  "Five",
  "Six",
  "Seven",
  "Eight",
  "Nine",
  "Ten",
  "Unterknave",
  "Oberknave",
  "King",
};

// as well as pesky "of": suit "Acorns" or "Clubs", not "of Acorns"
string[] suits = new string[] {
  "Hearts",
  "Bells",
  "Acorns",
  "Leaves",
};

string[] deck;

public Deck() {
  // All combinations of name and suits (cartesian join)
  // Avoid magic numbers like 13 (some games like preference has 8 cards in a suit)
  deck = name
    .SelectMany(n => suits.Select(s => $"{n} of {s}")) // <- space and "of" here
    .ToArray();

score 1 · Answer 3 · answered Oct 16 '18 at 12:29

1

suits[4] = "of Leaves";

should be

suits[3] = "of Leaves";

answered Oct 16 '18 at 12:29

jazb

5,498
6
37
44

score 1 · Answer 4 · edited Oct 16 '18 at 13:16

While you've already got a direct answer to your question, I can offer a few code improvements that will make your life much easier.

Whenever iterating an array with a for loop, Prefer i < array.Length over i < SomeHardCodedValue.
to populate the deck, don't use 4 different loops, use nested loops.
do not populate a new deck of cards when shuffling, instead, either create a copy of the existing deck and shuffle that, or simply shuffle the existing deck.
prefer array initializers over loops when the values are hard coded.

Combining all of these points into code, a better version would look like this:

class Deck
{
    // The next two arrays are static since they are always the same,
    // no point of making multiple copies of them for each instance of the class
    private static string[] name = new string[] 
    {
        "Ace", "Two", "Three", "Four" /* more of the same here*/
    };

    private static string[] suits = new string[]
    {
        "Hearts", "Bells", "Acorns", "Leaves"
    };

    // all the fields used in this class
    private string[] deck = new string[suits.Length * name.Length];
    private int lastDelt = 0;
    private Random rnd = new Random();

    public Deck()
    {
        // see how simple the constructor is now
        for(var i = 0 ; i < suits.Length; i++)
        {
            for(var j = 0; j < name.Length; j++)
            {
                // Calculate deck index using i and j
                deck[(i+1) * j] = name[j] +" of "+ suits[i];
            }
        }
    }

    public void Shuffle()
    {
        // You might want to look into the Fisher-Yates shuffle instead, link below.
        deck = deck.OrderBy(x => rnd.Next()).ToArray();

        cardsLeft = deck.Length;
    }

    public string Deal()
    {
        if(lastDelt >= deck.Length)
        {
            throw new Exception("No cards left in the deck");
        }
        string deltCard = deck[lastDelt];
        lastDelt++;
        return deltCard;
    }

}

Of course, you could have the entire deck in a single hard coded static array, and then use a copy of it in each constructor.

A link to a c# implementation of the Fisher-Yates shuffle algorithm, as promised.

`"of Hearts"`, `"of Bells"` is not a good choice: what if I want to enumerates suites? E.g. `Console.Write("Choose suit: {string.Join(", ", suits)}");`. Let `suits` be *suits*: `"Acorns"`, `"Bells"` etc. — Dmitry Bychenko, Oct 16 '18 at 13:01
`52` is a *magic number* in the `private string[] deck = new string[52];`: what if I'll want to implement *preference* card game with `32` cards deck? Better initialize `deck` in the constructor: `deck = new string[name.Length * suits.Length];` — Dmitry Bychenko, Oct 16 '18 at 13:01

Index out of range.... but it doesn't appear to be out of range so whats going on?

4 Answers4