3

I have a bit of an issue with my program. I have a function void loadData() which will load the data from a text file customers.txt and store each line of data into a Linked List. My concern is, specifically with how I/O works. I managed to get the data from the text file into and stored into a linked list data member variable. When i call that variable i get the answer i want printed onto the console. std::cout << "Group Name: " << tempCustomer->groupName << std::endl;

However, i decided to run a console output command later in the function to test if all the variables had the right data, i realize that it was all over the place. I'm not sure why its not working.

Here is the loadData() function

void Groups::loadData(){
  fin.open("customers.txt"); 
  char holder[MAX_SIZE];

  if(!fin.is_open())
    std::cerr << "Could not access file" << std::endl;
  else{
    while(!fin.eof()){
        Customers *tempCustomer = new Customers;

        fin.getline(holder,MAX_SIZE,';');
        tempCustomer->groupName = holder;

        std::cout << "Group Name: " << tempCustomer->groupName << std::endl;
        fin.getline(holder,MAX_SIZE,';');
        tempCustomer->name = holder;

        fin.getline(holder,MAX_SIZE,';');
        tempCustomer->email = holder;


        fin >> tempCustomer->choice;
        fin.get(); //gets the last character, which is '\n'
        fin.ignore(); //ignores the next character which is the '\n'

        tempCustomer->next = NULL;

        std::cout << "What does the temp Node Store?" << std::endl;
        std::cout << "Group Name: " << tempCustomer->groupName << std::endl;
        std::cout << "Name: " << tempCustomer->name << std::endl;
        std::cout << "Email: " << tempCustomer->email << std::endl;
        std::cout << "Choice: " << tempCustomer->choice << std::endl;

        //addCustomerToLL(tempCustomer);
        tempCustomer = NULL;
        delete tempCustomer;

    }    
   }
   fin.close();
  }

Here is the Console out put:

Group Name: Jonathan Group
What does the temp Node Store?
Group Name: vazquez.jonathan@pcc.edu
Name: vazquez.jonathan@pcc.edu
Email: vazquez.jonathan@pcc.edu
Choice: 2

Here is the text file customers.txt

Jonathan Group;Jonathan;vazquez.jonathan@pcc.edu;2

This is a school assignment, i'm to store all the customers from the text file into a linked list. I'm also to use c strings as strings rather than c++ version of strings. Let me know if the other files are necessary, i didnt include them since well nothing in this function utilize anything else outside the func besides the ifstream fin; private variable i have in the class and the const int MAX_SIZE = 256; global variable.

  • what's `MAX_SIZE`? – Joseph D. May 07 '18 at 00:34
  • What's a `Customers` ? Edit your question to include a [mcve]. – Sid S May 07 '18 at 00:41
  • @codekaizer MAX_SIZE is set to 256. I wrote it at the bottom of the thread. I have it since `char holder[MAX_SIZE];` will contain the string from the text file that is been read. – Jonathan Vazquez May 07 '18 at 00:42
  • @sidS `Customers` is the data type of my struct located inside a class private. It has 4 variables, 3 which are char pointers and 1 which is an int. The other variable is simply a pointer to the next node (linked list). – Jonathan Vazquez May 07 '18 at 00:44
  • @JonathanVazquez - "'im also to use c strings as strings rather than c++ version of strings", WHY? – Joseph D. May 07 '18 at 00:48
  • @codekaizer , trust me i argued with the instructor on why, but like every university they want to keep utilizing C methodology in C++ even though C++ was made as an improvement of C, not vice-versa. So sadly enough i have to remain using it. – Jonathan Vazquez May 07 '18 at 00:51
  • I know right! It's a terrible way to teach but almost every class uses it. You should get a [good book](https://stackoverflow.com/q/388242/9254539) if you don't have one already. Other than that, try using a debugger to step through the code. – eesiraed May 07 '18 at 00:54
  • @Fei Xiang yeah sad, i have literally 4 books sitting here. None of them provide me a good understanding of what im having issues with. I just dont seem to understand why the last char pointer is the only value been outputted. As for the debugger, well i was using gdb but then im not having much luck with it (i need more debugging practice). – Jonathan Vazquez May 07 '18 at 00:56
  • One of the big pitfalls of char arrays is copying strings. `tempCustomer->groupName = holder;` just copies the pointer, not the actual string. You're going to need to somehow allocate memory for stuff like the `groupName` of each customer object. Then you would copy the string with `strcpy`. – eesiraed May 07 '18 at 01:01
  • `while(!fin.eof())` this looks wrong. This tells you whether or not you've reached the end of the file, not whether or not the next `getline` call will succeed. You need to check the return value of `getline` instead. – MFisherKDX May 07 '18 at 02:04
  • @MFisherKDX most of my books i have do `while(!fin.eof())` rather than another way. (fin is object to ifstream) – Jonathan Vazquez May 07 '18 at 03:18
  • https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong – MFisherKDX May 07 '18 at 03:20

2 Answers2

2

Assuming you're not allowed to use std::string, you need to allocate memory for each string.

So replace this:

fin.getline(holder,MAX_SIZE,';');
tempCustomer->groupName = holder;

with:

fin.getline(holder, MAX_SIZE, ';');
char *s = new char[strlen(holder) + 1];
strcpy(s, holder);
tempCustomer->groupName = s;

You should release the memory you allocate when you no longer need it, so create a destructor for your Customers class:

Customers::~Customers()
{
    delete[] groupName;
}
Sid S
  • 6,037
  • 2
  • 18
  • 24
  • So that worked. Is the reason why `tempCustomer->groupName = holder` does not work is because we have a char pointer = a char array? As for the destruct, since this is for a temporary Node, i'm assuming i just delete the entire node within the function after using it. Do i have to delete the `char *s` as well or does it automatically delete when i delete the entire temp Node? – Jonathan Vazquez May 07 '18 at 01:02
  • holder which is `char[MAX_SIZE]` will decay to `char *` – Joseph D. May 07 '18 at 01:06
  • Two problems with using `holder` like you did - 1) There is only one of it, but you keep reusing it and store several pointers to it. They all point to the same thing, which will be the last thing stored there. 2) `holder` goes out of scope when `Groups::loadData()` returns. After that, accessing the pointers to it will be undefined behavior. – Sid S May 07 '18 at 01:11
  • You do not have to `delete[] s;` - because you created a destructor for `Customers` that does that. – Sid S May 07 '18 at 01:16
  • @SidS perfect so now it works. I left the `char holder[MAX_SIZE];` as it is since my application is working good. I managed to enter more entries into the text file and add them all into a linked list and display the Linked list. It all outputted perfectly the way i wanted. Out curiosity, do you recommend that i pass the tempCustomer node as a parameter in which it will add the node into the HEAD node? So far i added all the code within a single function (loadData) which works so far. – Jonathan Vazquez May 07 '18 at 01:35
  • @SidS never mind. I simply did this `addCustomerToLL(Customers *&temp);` which allows me to have a separate function that simply adds the contents from a text file into a linked list. – Jonathan Vazquez May 07 '18 at 01:39
1

It's because the holder changes when you read a new line,but your all string in your Customer points to the same holder which stores the last line you read. Change the type of name,email etc to char[MAX_SIZE] may help.

  • Per my teachers instruction she wants us to use pointer to char for those member variables within a struct (Linked List). Is there a way for me to perhaps reset holder that way i can keep reusing it? – Jonathan Vazquez May 07 '18 at 00:36
  • So dynamic memory will be needed.Just like pointer = (char*)malloc(strlen(holder)+1);memcpy(pointer,holder);And never forget to free all the pointers and add a '\0' to the tail of holder because the functions my need it. –  May 07 '18 at 00:43
  • dynamic allocation? Could you provide an example of what you mean? what am i exactly adding dynamic memory powers to? Is it holder or the pointers of for example `tempCustomer->groupName`? – Jonathan Vazquez May 07 '18 at 00:52
  • I'm sorry for my poor English.'tempCustomer->groupName = (char*)malloc (strlen(holder));' . Thus the groupName points to a dynamic allocated memory and then copy the 'holder' to the 'groupName'. –  May 07 '18 at 00:57
  • @吴泽平, no need to cast to `char*`. see [this](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) post. – Joseph D. May 07 '18 at 01:01
  • @codekaizer Thank you ! I get it. I mainly use c++ so it is naturely for me to do a cast. –  May 07 '18 at 01:06