Trouble reading from file?

Hi all,

I have built on the file Kurt helped me with, it is supposed to write values out to a file after the list is modified (which it does), but will not read/assign properly from the written file. To give you an example, one of my files from which I read has a, b, c, d, and e written to it, but on separate lines. When I read from the file, I also cout the values as they are read, to try to figure out where it goes wrong, and it reads a, b, c, and stops, then when I try to assign them to the info portion of the nodes, it does no assigning at all. I'm so close to being done, could someone clue me in on what is going wrong here?

Thanks!

-Patrick

.cpp code attached, but most relevant function below

listFromFile function:
__________________________________________________ ______________

Code:

---


void linkedList::listFromFile(ifstream& inputFile)//get node->value for all nodes from chosen file
{
        char valPortion[255];
        string valuePortion;//value to be put into node->value for all nodes
            for(int iTwo = 1; iTwo <= 5; iTwo++)
                {
                        (getline(inputFile, valuePortion));//while eof marker not found in file, read until
                        iTwo++;
                        cout << valuePortion;
                        strcpy(valPortion, valuePortion.c_str());//char *valPortion = strdup(valuePortion.c_str())  copy line from file into valPortion, which will be put into new node
                    //linkedListNode node(valPortion);//create node w/value = line of text
            //add(node, iTwo);//add the above-created node to list
                            linkedListNode node(valPortion);//access node to be deleted(overwritten)
                                        add(node, iTwo);                       
                }
       
}

---

Whole .cpp file attached

OK, part of it was that I incremented iTwo twice in the loop (silly me), but it still won't write the strings I pull from the file to the value portion of the nodes. Why?

-Patrick

Code:

---

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

class linkedListNode
{
       
      public:
           
                    char *value; //char array info portion of node
            linkedListNode *nextPtr; //declare pointer to next node in list
            linkedListNode(char *val = "", linkedListNode *nxtPtr = 0) //node constructor, default empty string for info portion, 'value'
                        {
                                nextPtr = nxtPtr;
                                value = strdup(val);
                        }
                        linkedListNode& operator=(const linkedListNode&); //explicit copy constructor, due to pointers
                        linkedListNode(const linkedListNode&);//explicit assingment operator, due to pointers
                        ~linkedListNode();//explicitly declared destructor, due to pointers
                                       
};
                         

class linkedList
 {
      public:
            linkedList() //constructor setting nodes = 0
            {
                        headPtr = tailPtr = 0;//head and tail = 0
            }
            ~linkedList();//explicit destructor
            void add(const linkedListNode& node, int);//function to add a node, used for both createEmptyList, and to add/replace a node
                        linkedListNode * get(int idx);//uses temp variable to locate node for any function that accesses node in the list
                        void insertLine(int);//insert(or replace) text in a particular node
                        void display();//displays list for clearer, more user-friendly interface
            void deleteNode();//sets 'value' portion of node = "", doesn't actually delete, but as far as end-user is concerned, it does
                        void deleteLines();//see above deleteNode() comments, "deletes" multiple lines
                        void addTextToLine();//appends text to 'value' portion of nodes
            void createEmptyList();//creates empty nodes for user to manipulate at beginning of int main()
            void listFromFile(ifstream&); 
    private:
            linkedListNode *headPtr, *tailPtr; //head and tail pointers for linked list
};

linkedListNode::linkedListNode(const linkedListNode& l)//copy constructor
{

  nextPtr = l.nextPtr; //copy pointer over
  value = strdup(l.value);//copy value field over
}
linkedListNode& linkedListNode::operator=(const linkedListNode& l) //assignment op.
{
        if(this != &l) //make sure not to assign to itself
                {
                if(value != 0) //if value portion already has a value assigned
                {
                free(value); //delete it
                }
                value = strdup(l.value); //copy existing value to freed field
        nextPtr = l.nextPtr; //copy existing pointer to l-value pointer field
                }
    return *this; //std. w/ass. op.
}
linkedListNode::~linkedListNode()
{
    if(value != 0)//if value field has a value
    {
                value = 0;//to prevent dangling reference problem
    }
}                       

linkedList::~linkedList() //linkedList destructor
{
    while(headPtr)//as long as there are more nodes
    {
      linkedListNode *p = headPtr;//temp variable to traverse list while deleting sequentially
          headPtr = headPtr->nextPtr;//make head = next element
          delete p; //delete temp, which is old head
    }
}

void linkedList::add(const linkedListNode& node, int i) //add/replace blank/existing on line i text w/node->value
{
        if(headPtr == 0) //if empty
        {
                headPtr = new linkedListNode(node); //node = head
                tailPtr = headPtr;//node also = tail
        }
        else //if not empty
        {
                linkedListNode *current = new linkedListNode(node); //create a tmp node with values of 'node' passed to function
                current = headPtr; //start at the head of the list
                static int count = 0; //set static counter to 0
                while(current->nextPtr) //while nextPtr (next element) exists
                { 
                        count++; //increment counter (start at 1, since first element = 1)
                    if(count == i) //if line number == counter
                        {
                                break; //break out of the loop, because this is the line number we want to insert/overwrite
                        }
                        else // if (count != i)
                        {
                        current = current->nextPtr; //move on down the list, one element at a time
                        }
                }
        int j; //new counter int
                for(j = 0; j <= i-count; j++) //if sequence wasn't completed with i-loop
                {
                tailPtr->nextPtr = new linkedListNode(); //go on creating empty nodes
                tailPtr = tailPtr->nextPtr; //set tail = to new node just created at end
                }
        }
}


void linkedList::createEmptyList()//this is done to make nodes available for editing to user
{
        if(headPtr)
        {
                return;
        }
        linkedListNode node("");        // create an empty node on the stack
        for(int i = 1; i < 4; i++)      // put 3 copies into the list
        {
                add(node, i);//add node into position i of list
        }

}

void linkedList::addTextToLine()//function for appending data
{
        int lineNum;//line number which is to be appended to, input by user
        cout << "Which line would you like to append to?" << endl;
        cin >> lineNum;//input line number to which data is to be appended
        char appTxt[255];//text to be appended
        cout << "Enter the text to be appended to line " << lineNum << endl;//prompt user to input text
        cin.getline(appTxt, 255);//accept text
    linkedListNode *tmp;//create temp node to traverse list to find desired line
        tmp = headPtr;//start at beginning of list
    for(int i = 1; i <= lineNum; i++)//for loop to go to desired line
    {
          tmp = tmp->nextPtr;//keep incrementing until desired node found
    }
    strcat(tmp->value, appTxt);//append text to line after desired line found w/for loop
}

void linkedList::listFromFile(ifstream& inputFile)//get node->value for all nodes from chosen file
{
        char valPortion[255];
        string valuePortion;//value to be put into node->value for all nodes
            for(int iTwo = 1; iTwo <= 5; iTwo++)
                {
                        (getline(inputFile, valuePortion));//while eof marker not found in file, read until
                        cout << valuePortion;
                        strcpy(valPortion, valuePortion.c_str());//char *valPortion = strdup(valuePortion.c_str())  copy line from file into valPortion, which will be put into new node
                    //linkedListNode node(valPortion);//create node w/value = line of text
            //add(node, iTwo);//add the above-created node to list
                            linkedListNode node(valPortion);//access node to be deleted(overwritten)
                                        add(node, iTwo);                       
                }
       
}
       

void linkedList::display()//display list to user before modifying, for more user-friendliness
{
        linkedListNode *node = headPtr;//new temp node to traverse list, starts at head because will go sequentially displaying values for user
        int count = 1; //set counter to 1 and display and increment
        while(node)//while there is still a node to display
        {
                cout << count++ << ": " << node->value << "" << endl;//cout value and increment counter for next node
                node = node->nextPtr;//move on down the list
        }
}

linkedListNode * linkedList::get(int count)//function to go to desired element, to make manipulation possible
{
        linkedListNode *pnode = headPtr;//new temp object to find desired node
        while(count--)//easier than for-loop
        {
                pnode = pnode->nextPtr;//go on to next node until nth node
                if(!pnode)//if end of list
                {
                        break;//stop
                }
        }
        return pnode;//get(return) the nth node
}
int main()
{
    linkedList l;
    l.createEmptyList();
    fileGoto:
        ifstream inputFile;//input file stream object
        ofstream outputFile;//output file stream object
        string filename;//name of file to be input by user
        cout << "EDIT: enter file name: ";//prompt user for name of file
        cin >> filename;//user enters name of file to be written
        try//get ready to throw fake exception (skips rest of block, which not necessary if file non-existent) if file not opened for input due to non-existence
        {
            inputFile.open(filename.c_str());//open the chosen file
            if(inputFile.fail())//if file does not exist
                    throw 1;//throw so as to not execute below statements
            char overWrite;//y or n overwrite prompt to user
            cout << "A file by that name already exists. Would you like to overwrite it? ('y' or 'n'): ";
            cin >> overWrite;//user inputs yes or no, y or n
            cin.ignore();//flush buffer
            if(tolower(overWrite) == 'y')
            {
                    //linkedList l;//create linkedList object l
                    l.listFromFile(inputFile);//get list from file and set data in file = node->value for all of the nodes
                    goto ifFromFile;//had this set up so as to not declare linkedList l twice, but didn't like me skipping instantiation w/control statement
            }
            else if(tolower(overWrite) == 'n')
            {
                inputFile.close();//close file up
                    cout << "OK, file will not be overwritten";
                    goto fileGoto;//go back to file prompt
            }
        }
        catch(int e) {};//do nothing with artificially thrown exception
        cin.ignore();//flush buffer after cin statement, since cin and getline used in same program
        //create a linkedList object to use linkedList functions
    //create empty list to make possible user manipulation of nodes
    ifFromFile:
        int x = 2;//used to create infinite loop(see use of 'while(x < 4)' below
    here://if user inputs invalid data in list options, will cout error message, then return control here
        char selection;//for operation to perform
        cout << "Which option would you like to perform? " << endl;
        cout << "'i' to insert new data or replace existing data;" << endl;
        cout << "'d' to delete existing data;" << endl;
        cout << "'a' to append to data;" << endl;
        cout << "'e' to exit program;" << endl;
        cout << "Enter your selection: ";
                cin >> selection;//user selects operation to perform
                cin.ignore();//get rid of buffer, because I'm using cin and getline in same program
                if(tolower(selection) == 'i')
                {
                    while(x < 4)//here's where I create an endless loop until 'n' entered, then use control variable to move back to options list
                {
                          char data[256];//for data to be inserted
                      int i;//line number to be entered
                                  char response;//y or n response to insert prompt
                  l.display();//display list for user-friendliness
                  insGoto://control statement to return to if invalid data input (after warning message displayed)
                  cout << "Would you like to insert/replace data? ('y' or 'n'): ";
                      cin >> response;//y or n
                      cin.ignore();//flush buffer
                      if(tolower(response) == 'n')
                              goto here;//if no more data to be entered, go back to options list
                  else if(tolower(response) == 'y')//if data to be entered
                      {
                      cout << "Enter the line number you would like to insert/replace: ";
                          cin >> i;//user inputs line number
                                          cin.ignore();//flush buffer
                                          cout << endl;
                          linkedListNode *pnode = l.get(i-1);//go to the desired node
                              if(pnode)//if node exists
                              {
                                      cout << "Adding/replacing node " << i << " : ";                           
                                      cin.getline(data, 256);//get data to be inserted
                                          *pnode = linkedListNode(data, pnode->nextPtr);//write new data to node
                              }
                      }
                                  else//if y, Y, n or N not input
                                  {
                                          cout << "Sorry, invalid input.  Please answer 'y' or 'n'." << endl;
                                          goto insGoto;//go back to input/replace prompt
                                  }
                }
                }
              else if(tolower(selection) == 'd')//if delete desired
                {
                while(x < 4)//create endless loop user can break out of by entering 'n'
                {
                    l.display();//display contents of list for user-friendliness
                    char delYN;//y or n if they want to delete node
                delGoto://control statement to go to if invalid delYN response entered
                    cout << "Would you like to delete any of this info? (y or n): ";
                cin >> delYN;//user decided yes or no
                    if(tolower(delYN) == 'n')
                            goto here;//if no more info to be input, go back to main menu
                    else if(tolower(delYN) == 'y')
                    {
                            char multLines;//user decides if just 1 line, or more than one to be deleted
                            cout << "Would you like to delete 1 line, or multiple (1 or m)?: ";
                            cin >> multLines;//1 or more, user picks
                            cin.ignore();//flush buffer
                        if(multLines == '1')//if only 1 line to be deleted
                            {
                                int nodeNum;//line/node to be deleted
                                cout << "Enter the node you would like to delete: ";//prompt user
                                cin >> nodeNum;//user input
                        linkedListNode *dnode = l.get(nodeNum-1);//access node to be deleted(overwritten with "")
                                    if(dnode)//if node exists
                                    {
                                            cout << "Node " << nodeNum << " : "
                                                      << dnode->value << "" << " deleted ";
                                            dnode->value = "";//overwrite with "", so user can still access node for manipulation
                                            cout << endl;
                                    }
                            }
                        else if(tolower(multLines) == 'm')//if more than one line is to be deleted
                        {
                                int startLine, endLine;//line to start deleting, line to stop deleting
                                cout << "Where do you want to start deleting, which line?: ";
                                cin >> startLine;//start here
                                cin.ignore();//flush buffer
                                cout << "Where do you want to stop deleting, which line?: ";
                                cin >> endLine;//stop here
                                cin.ignore();//flush buffer
                                for(int k = startLine; k <= endLine; k++)//for loop to delete(overwrite) desired lines
                                    {
                            linkedListNode *dnode = l.get(k-1);//access node to be deleted(overwritten)
                                        if(dnode)//if node exists
                                            {
                                                    dnode->value = "";//overwrite it - keep it for user access later, don't delete completely
                                                    cout << endl;
                                            }
                                }
                        }
                                else//if neither y nor n input
                                {
                                        cout << "Sorry, that was incorrect input.  Please enter either y or no." << endl;
                                        goto delGoto;
                                }

                                }
                        }
                }
                else if(tolower(selection) == 'a')//if append(concatenate) operation desired
                {
                while(x < 4)//endless loop only user breaks out of by selecting 'n'
                    {
                        l.display();//display list again for user-frienliness
                        char appYN;//for deciding whether or not to continue w/append operation
                    appGoto://goto control statement if user inputs invalid data (neither y nor n)
                        cout << "Would you like to append to any of this info? (y or n): ";
                        cin >> appYN;//y or n
                        if(tolower(appYN) == 'n')
                                goto here;//go back to main menu if done appending
                        else if(tolower(appYN) == 'y')
                            {
                            cout << "Enter the line number to which you would like to append?: ";
                                int appLine;//line to be appended
                                cin >> appLine;//user inputs line #
                                cin.ignore();//flush buffer
                                cout << endl;
                                char appTxt[255];//text to be concatenated
                                linkedListNode *pnode = l.get(appLine-1);//access node to which we wish to append text
                                if(pnode)//if node exists
                                {
                                        cout << "Enter the text to be appended to line " << appLine << ": ";
                                        cin.getline(appTxt, 255);//user inputs text to be concatenated
                                            strcat(pnode->value, appTxt);//concatenate text to value portion of node
                                    }
                            }
                                        else//if neither y nor n entered to appendText prompt
                                        {
                                                cout << "Sorry, wrong input.  Please enter 'y' or 'n'." << endl;
                                                goto appGoto;//go back to appText y or n prompt
                                        }
                    }
              }
              else if(tolower(selection) == 'e')//if exit from program desired
              {
              outputFile.open(filename.c_str());//open outfile object for writing to chose file
              for(int n = 1; n <= 5; n++)//for-loop to write out each node's info portion to file
                          {
                          linkedListNode *onode = l.get(n-1);//access node to be deleted(overwritten)
                                        if(onode)//if node exists
                                            {
                                                    outputFile << onode->value << endl;//write the value portion the the file for access next time opened
                                                   
                                            }                                                       
                      }
                          outputFile.close();//close file before exiting program
                      exit(1);//exit program
              }
                  else//if none of options from main menu chosen
                  {
                          cout << "Sorry, wrong input.  Choose a selection from the menu " << endl;
                          goto here;//go back to main meun
                  }
                  return 0;
}

---

I haven't analyzed your program extensively but I suspect the problem is here:

char *value; //char array info portion of node

value is not a char array. value is a pointer. In order to make value act like an array you need to have value point to memory associated with the first char in a string. In a situation like this, I might do something like this:

Code:

---

//put inputString into a new linkedListNode and add the new linkedListNode to the linkedList
void linkedList add(char * inputString)
{
    linkedListNode newNode = new linkedListNode;
    newNode->value = new char[strlen(inputString) + 1];
    strcpy(newNode->value, inputString);
     
    //code to add newNode into a linkedList goes here
}

---

Within main() or whatever, I would obtain inputString by whatever mechanism you want.

The linkedListNode destructor should release the memory assigned to value.

By the way, I think a loop containing the menu and a switch statement using the value of selection to control user access to the linkedList's methods would work better than the style of code you have written, but that has nothing to do with question you asked.

Code:

---

   
for(int iTwo = 1; iTwo <= 5; iTwo++)
                {
                        (getline(inputFile, valuePortion));//while eof marker not found in file, read until
                        iTwo++;

---

You increase your counter twice in every iteration
so the loop is processed for iTwo == 1, 3, 5 is that what you planned to do?

Yeah, I did catch the increment error, as my second post in this thread indicates. I just reused:

Code:

---

for(int iTwo = 1; iTwo <= 5; iTwo++)//loop to copy file lines to all elements of list
                {
                        (getline(inputFile, valuePortion));//while eof marker not found in file, read until
                        strcpy(valPortion, valuePortion.c_str());//char *valPortion = strdup(valuePortion.c_str())  copy line from file into valPortion, which will be put into new node
                       
                          linkedListNode *pnode = get(iTwo-1);//go to the desired node
                          *pnode = linkedListNode(valPortion, pnode->nextPtr);//write new data to node
                }

---

, and it worked. Can anyone tell me what was wrong with

Code:

---

for(int iTwo = 1; iTwo <= 5; iTwo++)
                {
                        (getline(inputFile, valuePortion));//while eof marker not found in file, read until
                        cout << valuePortion;
                        strcpy(valPortion, valuePortion.c_str());//char *valPortion = strdup(valuePortion.c_str())  copy line from file into valPortion, which will be put into new node
                    //linkedListNode node(valPortion);//create node w/value = line of text
            //add(node, iTwo);//add the above-created node to list
                            linkedListNode node(valPortion);//access node to be deleted(overwritten)
                                        add(node, iTwo);                       
                }

---

Thanks!

-Patrick

Quote:

---

Can anyone tell me what was wrong with

---

What is the diagnosis?

Umm.

Why are you making a temporary character array (valPortion) to duplicate the contents of valuePortion when you could just pass valuePortion.c_str() to your add() function (assuming you fix the parameter to be const-correct, which it is not.

For that matter, why are you using C-style strings at all? Make everything std::string -- inside your node, as a parameter to add(), etc.

You also have a lot of memory leaks (again, C++ strings would completely solve this problem). In your constructor and assignment operator, you allocate memory using strdup. In add, you allocate memory using new[]. None of that memory is ever freed, and because you don't even know which method allocated that memory, it's not even POSSIBLE to free it. How do you know if you're supposed to call free() or delete[] on that memory?

If you used a C++ string as the data storage you wouldn't need to worry about that.

Cat,

Thanks for the constructive criticism. When you say C++ string, are you referring to std:string, just a normal string variable? When you say C-style strings, I guess you mean c-strings? Thanks again, I don't know much about which methods/procedures are more/less efficient, or how to avoid memory leaks; I just kind of got thrown into this data structures class after 2 basic C++ courses, and it's almost like a self-study in that the book assumes we already know these things.

-Patrick

C-strings are char * or const char * strings; the only kind of strings that existed in the C programming language which C++ inherited.

When you use them, you need to manage their memory carefully. You need to allocate enough space for the string to "live" in, you need to give that space back to the operating system when you're done with it, etc.

C++ strings are std::string variables; the preferred C++ method of storing strings. They internally will use some kind of char array(s), but they manage all the allocation and deallocation themselves. There's no worrying about delete[] or free() or anything like that; it's all done inside the string's destructor. They should be used in almost every case instead of C-style strings.

Good stuff, Cat, this is nowhere to be found in my texts; I didn't know about the convenience of C++ strings. Thanks again, I appreciate the advice/explanation!

-Patrick