Programming question for c#

[Mr Hat and Clogs]

(Not sure where to put this, since it isn't quite a 'tech support' question, so general it is?)

Ello!

So I'm brand spanking new to this whole programming thing (tbh I feel like idiot half the time forgetting things), and I'm having some trouble with something hopefully someone here could help me with.

The situation is this, I have a file I need to read/load data from and stick into a struct, and rather than being in a nice sequential list with 1 item per line its all in one string. (ie. C 12 D 12 H 3 D 5 S 6)

So what I have done is split it temporarily into an array (parts), than convert the string into an int and char, and load it into the struct (I have to use a struct) for future use.

I'm positive there is a better way to go about this than the ass backwards method I'm using, so if anyone has an idea it would be appreciated.

I've gone through a couple variants of this, but this is the one I have atm.

here is the parts I'm having trouble with:

  List<Card> info = new List<Card>();
                    foreach (string line in File.ReadAllLines(fileName))
                    {
                        string[] parts = line.Split(' ');

                        for (int index = 0; index < currentCard.Length; index++)
                        {
                            currentCard[index].firstSuit = Convert.ToChar(parts[0]);
                            currentCard[index].firstFace = Convert.ToInt32(parts[1]);

                            //testing
                            Console.WriteLine(currentCard[index].firstSuit);
                            Console.WriteLine(currentCard[index].firstFace);


                        }

I guess the main issue is that when ever I change "Convert.ToChar(parts[0]);" to anything else like "Convert.ToChar(parts[index]);"
things go tits up. So if anyone smarter than I knows a way around this, it'd be MUCHLY appreciated.

This is a different way I tried, but I always seem to get errors when converting string to int or char. The above is the only one that 'seems' to have worked, but only for the first two array elements. Here's a couple extra bits, but bare in mind I've changed some of the names of things.

Spoiler for extra's:

if (File.Exists(fileName))
                {
                    StreamReader inFile = new StreamReader(fileName);
                    Console.WriteLine(inFile.ReadLine());
                    hand = inFile.ReadLine();
                    Console.WriteLine(hand);
                    while (hand != null)
                    {
                        cardsArray = hand.Split(' ');
                        hand = inFile.ReadLine();
                    }
                    foreach (string card in cardsArray)
                        Console.WriteLine(card);
                    

                    inFile.Close();
                }
                else
                    Console.WriteLine("File does not exist.");
and

if (File.Exists(fileName))
                {
                    StreamReader inFile = new StreamReader(fileName);
                    rawHand = inFile.ReadLine();
                    while (rawHand != null)
                    {
                        cardsArray = rawHand.Split(' ');
                        rawHand = inFile.ReadLine();
                        for (int counter = 0; counter < cardsArray.Length; counter++)
                        {
                            currentCard[counter].firstSuit = Convert.ToChar(rawHand);
                            //Convert.ToChar(cardsArray[0]);
                            //Convert.ToInt16(cardsArray[1]);

                        }
                    }

Here is a larger code block to give you more an idea of what I have going on. There is more after this that consists of methods, but I don't want to take up too much space.

Spoiler for code block:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;

namespace PokerHandTest
{
    class Program
    {
        public struct Card
        {
            //Suit (C = Club, D = Diamond, H = Heart, S = Spade) 
            //Face (2-14 – for 2-10,Jack, Queen, King, Ace)
            public char firstSuit;
            public int firstFace;

        }
        static void Main(string[] args)
        {
            char input = 'y';

            while (input == 'y' || input == 'Y')
            {
                //face & suit for each card
                const int CARD_NUMBER = 10;
                //array of the different file types.
                string[] file = {"Flush", "FourKind", "FullHouse", 
                                    "Pair", "RoyalFlush", "Straight", 
                                    "StraightFlush", "ThreeKind", 
                                    "TwoPair"};
                Card[] currentCard = new Card[CARD_NUMBER];
                string fileName;
                int fileNumber = 0;
                int count = 0;


                Console.WriteLine("There are nine files (1 - 9).");
                Console.WriteLine("\nType a number between 1 and 9" +
                    "you wish to evaluate.\n");
                Console.Write("\nSelection: ");

                //generates the file name
                fileNumber = Convert.ToInt16(Console.ReadLine());
                fileName = file[fileNumber - 1] + ".txt";


                if (File.Exists(fileName))
                {
                    List<Card> info = new List<Card>();
                    foreach (string line in File.ReadAllLines(fileName))
                    {
                        string[] parts = line.Split(' ');

                        for (int index = 0; index < currentCard.Length; index++)
                        {
                            currentCard[index].firstSuit = Convert.ToChar(parts[index]);
                            currentCard[index].firstFace = Convert.ToInt32(parts[index + 1]);

                            //testing
                            Console.WriteLine(currentCard[index].firstSuit);
                            Console.WriteLine(currentCard[index].firstFace);


                        }
                        //inFile.Close();
                    }

                }
                else
                    Console.WriteLine("File does not exist.");

                //continue or end program loop
                Console.Write("\nDo you wish to continue (Y/N): ");
                input = Convert.ToChar(Console.ReadLine());
                Console.Clear(); //resets to initial state if continuing.

            }
            EndProgram();
        }

Any help at all would be greatly appreciated.

[RobotCat]

Uh, how long are the 'lines' you're inputting? Your currentCard array is declared with a size of length = 10. In your for loop, your index goes from 0-9.

However, you blindly try to access parts[index] without first checking if that memory space is actually allocated. For example, in the sample line you gave, 'C 12 D 12 H 3 D 5 S 6' would return an array of 10 elements, however, based on your code, when your index is 10, you will try to access parts[10], which is actually the 11th element, which does not exist.

In fact, your mapping is incorrect too, as

currentCard[0].firstSuit is mapped to C
currentCard[0].firstFace is mapped to 12

but once you increase the index by 1

currentCard[1].firstSuit is mapped to 12
currentCard[1].firstFace is mapped to D

When it should be mapped to D, and then the next twelve. Ie, CurrenCard[0] should be mapped to parts[0] and parts [1]. CurrentCard[1] should be mapped to parts[2] and parts[3]. Instead of incrementing the index with index++, you should be incrementing by 2 every time, with index+= 2. This should get around the problem of exceeding the array allocation. However, that's assuming that you have at least 10 elements in the parts array. If it's dynamically sized, you will need to rewrite your loop based on the size of parts as opposed to the fixed length of currentCard.

Also, are you using an IDE of sorts? The debugger should tell you why it might be crashing. When you ask for help posting the error messages would be very useful.

[Jinto]

Have you actually made the stride length a multiple of two?

currentCard[index].firstSuit = parts[index*2][0];
currentCard[index].firstFace = Convert.ToInt32(parts[(index*2)+1]);

@RobotCat index += 2 doesnt work since you would generate empty entries in the currentCard List...
If the for loop was build differently though it would work:

for (int index = 0; index < currentCard.Length*2; index+=2)
{

Card newEntry;
newEntry.firstSuit = Convert.ToChar(parts[index]);
newEntry.firstFace = Convert.ToInt32(parts[index+1]);
currentCard.Add(newEntry);
}

I would also wrap a try-catch block around it checking for conversion Exceptions, because its not clear if your input data is always correct. Trying to convert a char into int might not work resulting in an Exception.
And as RobotCat mentioned already... the length of the parts array may not always be a mutliple of two of currentCard.Length, I would make a plausibility check before using it.

edit: I had to edit the for loop or only half the cards would have been read in (see red change). And I dont think that char needs conversion...

[Mr Hat and Clogs]

Using VS 2010.

The error it was throwing was FormatException, input string not in correct format.

Doesn't surprise me that it's completely botched, I've probably re-written in like a dozen different way's I've probably confused myself into a knot.

Went and had lunch to clear my head (not that it takes much to do that), came back and went back over some notes. Think I found a 'better' way to do what I want, but shall try what you've suggested. Was trying to see what I could do from memory mostly, probably not the best idea for a newb.

I always forget about try-catch as well, derp@me.

[Jinto]

Ah scratch my idea with the loop variant, because you already have a predefined currentCard array. Just go for the first thing with the stride length of two.

Besides, I would have written a line parser class that does all the magic of reading one line and used this to read files line by line in a file reader method in the main class.

Because you cannot optimize the way you read the file, but the line parsing is kind of a different concern (see design patter: "Seperation of Concerns")

edit: one more methodical thing I wanted to add

When you parse a line you should not constrain its length in the result array. But the length should be constraint by the actual input line. Something like

public List<Card> GetAllCardsInLine(string line)
{
 List<Card> allCardsInLine = new List<Card>();
 string[] parts = line.Split(' ');
 if(parts.Length%2 != 0)
 {
   Console.WriteLine("The line " + line + " cannot be parsed.");
   return allCardsInLine;
 }
 for(int i=0; i<parts.Length; i++)
 {
   if(i%2 == 0)
   {
      Card newEntry;
      newEntry.firstSuit = parts[i][0];  //this is already a substring simply access the char at index 0 (or does it use ASCII encoding or smth like that?)
      allCardsInLine.Add(newEntry);
   }
   else
   {
     try
     {
      allCardsInLine[i/2].firstFace = Convert.ToInt32(parts[i]);
     }
     catch(ConversionException ex)
     {
        Console.WriteLine("The line " + line + " cannot be parsed.");
        Console.WriteLine("Parsing the number: " + parts[i] + "at position " + i.ToString() + " threw an exception.";
        return allCardsInLine;
     }
   }
 }
 return allCardsInLine;
}

edit: I replaced the wrong code with parts[i][0]

[Mr Hat and Clogs]

Ended up going with atm:

                if (File.Exists(fileName))
                {
                    foreach (string line in File.ReadAllLines(fileName))
                    {
                        string[] parts = line.Split(' ');

                        for (int index = 0; index < parts.Length; index += 2)
                        {
                            currentCard[index / 2].firstSuit = Convert.ToChar(parts[index]);
                            currentCard[index / 2].firstFace = Convert.ToInt32(parts[index + 1]);
                            //testing
                            Console.WriteLine(currentCard[index / 2].firstSuit);
                            Console.WriteLine(currentCard[index / 2].firstFace);
                        }
                    }
                }

Seems to work alright but throws a hissy if the file's not correctly formatted. Prolly add some error catching to it.

With that code you've got there Jinto, whats the % covering? Division isn't it? You last comment, I think was what was throwing me for a while to.

[NightWish]

Originally Posted by Mr Hat and Clogs

Seems to work alright but throws a hissy if the file's not correctly formatted. Prolly add some error catching to it.

Jinto's post demonstrates how you might check for an even number of entries, one of the possible formatting issues which your code ignores silently. Erroneous characters in the stream will still throw OverflowException or FormatExceptions though.

Originally Posted by Mr Hat and Clogs

whats the % covering? Division isn't it?

Remainder after division, or "Modulo operation":

Originally Posted by msdn

The % operator computes the remainder after dividing its first operand by its second. All numeric types have predefined remainder operators.

[Jinto]

Originally Posted by Mr Hat and Clogs

...

With that code you've got there Jinto, whats the % covering? Division isn't it? You last comment, I think was what was throwing me for a while to.

The % is the modulo operator as stated by Nightwish.

My last comment (I dont know exactly what part of my post is meant) might be wrong, because I wrote code without Visual Studio - completely freestyle (I do not even know if a ConversionException exists, I assume not... but it was meant to be an example, not working code). And I messed up with the conversion of Int32.
When I finished the code, I realized that it is kinda strange to convert a string to char, since indexing over a string returns chars.
So I wanted to eliminate the conversion from string to char (using parts[i][0]). However I mistakenly replaced the Int32 conversion. And even made the comment on the wrong place.

Btw. if you think, that simply replacing the end-condition in the for loop does the trick - thats not enough. Because your currentCard array still has a predefined size. If you let the deserializer/parser decide the size (number of entries) of currentCard, than it would be correct.