Thread: Split function problem

Looking better in quite a few places!

Code:

        AddInfo >> lineBuffer;
        while(AddInfo.good())
        {
            getline(AddInfo, lineBuffer, '\n');
            string *theFields = split(lineBuffer, ',');
            cout << "SHOW RECORDS" << endl;
            cout << "---------------------------------" << endl;
            cout << "Name............ " << theFields[0] << endl;
            cout << "Street Address.. " << theFields[1] << endl;
            cout << "City............ " << theFields[2] << endl;
            cout << "State........... " << theFields[3] << endl;
            cout << "Zip Code........ " << theFields[4] << endl;
            cout << "---------------------------------" << endl;
        }

I'm afraid this doesn't fix the problem. AddrInfo.good() checks that there's no error and no EOF. So on the last valid run through the loop, getline fetches the last line. That doesn't set EOF, because it hasn't tried to read any further yet.
The loop will execute again -- getline will put the empty string "" in lineBuffer. split() will see that there are no commas, so only one (empty) substring.

Your function then tries to read from theFields[1] and onwards, but these don't exist. Hence crash. The loop needs to exit as soon as EOF is encountered by getline.

Really your code shouldn't be assuming that the array is a particular length. If all goes well, it will be, but if all does not go well, it's not ok for the program to crash. It'd be better to have split() also return the number of elements in the array. You could error and gracefully exit if it's not 5. To return an extra value from split() you could pass a count int as a reference.
If you were using vectors, you could just check vec.size().

You also still have this line before the loop:

Code:

        AddInfo >> lineBuffer;

This will just read the first word in the file, which could also crash your program depending on the contents of the file.

Just a style comment

Code:

ifstream AddInfo("C:\\Users\\User\\Documents\\Visual Studio 2012\\Projects\\Lab07\\Lab07\\TestAddress.txt", ios::in);

It'd be nicer to use a relative path here. Best would be to store TestAddress.txt alongside the executable, or in a directory next to the executable. Then the program could be run on different machines and would work correctly.
If you want to pick it up from your source dir though I think it's

Code:

..\\Lab07\\TestAddress.txt

It took me a few minutes of reading and rereading your comments smokey before it I got it, but I finally found the problem..

The code is now working as it should and more importantly I understand why it's working now. Thanks again for everyone's help.

For kicks, here's the final version.

Code:

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

void menu(void);
void writeData(void);
void readData(void);
string * split(string theLine, char theDeliminator);

const char FileName[] = "TestAddress.txt";
int main () 
{
    menu();
    return 0;
} 

void menu(void)
{
    char answer;
    do
    {
    cout << "Welcome to the Address Database." << endl;
    cout << "Do you wish to (A)ppend records, (D)isplay records, or (E)xit the program?" << endl;
    cin >> answer;
    cin.ignore();
    
        switch (answer)
        {    
            case 'A':; case 'a':;
            writeData(); 
            break;
            case'D': case 'd':;
            readData();
            break;
            case'E': case 'e':;
            exit (EXIT_SUCCESS);
            break;
            default:
            cout << "Invaild selection" << endl;
        }
    }
    while ((answer != 'E') || (answer != 'e'));
}


void writeData(void)
{
    char answer;
    
    ofstream AddInfo("TestAddress.txt",ios::app);

    if (AddInfo.is_open())
    {
        do
        {
            cout << endl;
            cout << "First and Last Name : " << endl;
            string name;
            getline(cin, name, '\n');

            cout << endl;
            cout << "Street Address: " << endl;
            string address;
            getline(cin, address, '\n');

            cout << endl;
            cout << "City: " << endl;
            string city;
            getline(cin, city, '\n');

            cout << endl;
            cout << "State: " << endl;
            string state;
            getline(cin, state, '\n');
    
            cout << endl;
            cout << "Zip Code: " << endl;
            string zip;
            getline(cin, zip, '\n');

            AddInfo << name << "," << address << "," << city << "," << state << "," << zip << endl;

            cout << endl;
            cout << "Enter Another Record? (Y/N)" << endl;
            cin >> answer;
            if (answer == 'N' || (answer == 'n'))
            {
                menu();
            } 
            cin.ignore();
        }
        while (answer == 'Y' || (answer == 'y'));
    
    }
    else
    {
        cout << "Could not open file." << endl;
    }
    AddInfo.close();
}

void readData(void)
{
int recordCount = 0;
string lineBuffer;
ifstream AddInfo("TestAddress.txt", ios::in);
     
    if (AddInfo.is_open()) // tests to see if file is open
    {
        cout << endl;
        cout << "SHOW RECORDS" << endl;
        cout << "------------" << endl;
        getline (AddInfo, lineBuffer); //gets first line of text and places it in "lineBuffer"
     
        while(AddInfo.good()) //while file is error free and not EOF
        {
            string * theFields = split(lineBuffer, ',');
            recordCount++;
             
            cout << "\n Record No. " << recordCount;
            cout << endl;
            cout << "Name............." << theFields[0] << endl;
            cout << "Address.........." << theFields[1] << endl;
            cout << "City............." << theFields[2] << endl;
            cout << "State............" << theFields[3] << endl;
            cout << "Zip.............." << theFields[4] << endl;
            cout << "------------------------------------------" << endl;
     
            getline(AddInfo, lineBuffer); //gets next line of text into "lineBuffer"
        }
     
        AddInfo.close();
        menu();
    }
    else
    {
        cout << "Unable to open file." << endl;
    }
}

string * split(string theLine, char theDeliminator)//Useage: string *theFields = split(lineBuffer, ',');
{
    int splitCount = 0;
    for(int i = 0; i < theLine.size(); i++)
    {
        if (theLine[i] == theDeliminator)
        splitCount++;
    }
    splitCount++;  
        
    string* theFieldArray; 
    theFieldArray = new string[splitCount]; 
        
    string theField = "";
    int commaCount = 0;

    for(int i = 0; i < theLine.size(); i++)
    { 
        if (theLine[i] != theDeliminator) 
        {
            theField += theLine[i]; 
        }
        else 
        { 
            theFieldArray[commaCount] = theField; 
            theField = "";
            commaCount++;
        }
    }
theFieldArray[commaCount] = theField; 

return theFieldArray;
}

Great Glad you worked it out for yourself. That way of doing it is fine.

Previously I said:

Originally Posted by smokeyangel

There's a very C++y way to do it, but the way it works is somewhat beyond your current knowledge, so it's probably best to go for a simpler approach. Or google until you find the answer Then read this: How does that funky while (std::cin >> foo) syntax work?, C++ FAQ

The slightly "magic" C++ way I was going to suggest was:

Code:

        cout << endl;
        cout << "SHOW RECORDS" << endl;
        cout << "------------" << endl;
              
        while(getline (AddInfo, lineBuffer)) //while file is error free and not EOF
        {
            string * theFields = split(lineBuffer, ',');
            recordCount++;
              
            cout << "\n Record No. " << recordCount;
            cout << endl;
            cout << "Name............." << theFields[0] << endl;
            cout << "Address.........." << theFields[1] << endl;
            cout << "City............." << theFields[2] << endl;
            cout << "State............" << theFields[3] << endl;
            cout << "Zip.............." << theFields[4] << endl;
            cout << "------------------------------------------" << endl;     
        }

getline() returns the same istream it read from, i.e. returns the first argument, AddInfo. This needs to be turned into a bool. Turns out that istream has "operator bool" to do just this. basic_ios:perator bool - C++ Reference

Looking more carefully I see that this will evaluate to false only if an error has occured. EOF isn't an error, it's just EOF. So don't use this Even if it evaluated to false for EOF, I'd not recommend using it unless you were completely happy you understood it. I don't understand why operator void *() is used to convert to bool, so I wouldn't risk bugs by using somethign I don't understand.

I'd probably do this:

Code:

        while(getline (AddInfo, lineBuffer).good()) //while file is error free and not EOF