Link List linking problem

A few things:

What does this->newNode(here); do?
Normally I would assume this call creates a new bookNode next to the bookNode here is pointing to, but I want to see what it's doing.

You never assigned here to the head of the list anyway. Or have you?

let's take a better look at the code. if you could post it like this in the first place it's would be more helpful.


bool llistBooks::loadFromFile(string fileName)
{
     ifstream in;
     in.open(fileName.c_str());

     if(!in)
     {
          cout << "There was a problem loading files";
          return false;  //this could be useful.
     }
     else
     {
          string Iprice, Icost;
          double the_double;
          bookNodePtr here = new bookNode();   //you missed the parenthases

          while(!in.eof())
          {
              getline(in,here->title,'\n');
              getline(in,here->author, '\n');
              getline(in,here->publisher,'\n');
              getline(in,here->isbn, '\n');
              getline(in,Iprice, '\n');
              the_double = strtod(Iprice.c_str(), NULL);
              here->price = the_double;
              getline(in,Icost, '\n');
              the_double = strtod(Icost.c_str(), NULL);
              here->cost = the_double;
              this->newNode(here);
          }
          here = NULL;
          delete here;
     }
     in.close();
     return true;
}


ok, looking at this code you have no error. the error may be in llistBooks::newNode(bookNodePtr)
or it might be where ever you initiate the linked list.

The head of the list was assigned prior to calling this function.
It was assigned with a default constructor.  This is the empty node that I mentioned.

The newNode function is:
void llistBooks::newNode(bookNodePtr& here)
{
here = new bookNode;                        
here->next = head;            
head = here;                              
}

Hi

Your newNode function is incorrect. The newNode function creates a new here overwriting the old one. Hence the new 'here' will be empty as you are getting it to be.

It should be something like:

void llistBooks::newNode(bookNodePtr& here)
{
bookNodePtr temp = new bookNode();                    
//copy all values to new node created
temp->title = here->title;
temp->author = here->author;
temp->publisher=here->publisher;
temp->isbn=here->isbn;
temp->price=here->price;
temp->cost=here->cost;
temp->next = head;
head = temp;
}

Dhyanesh

Hi

Another alternative is not to create the new node in function newNode but use the created node 'here' in loadFromFile as part of your link list.

i.e. your newNode function would then be:

void llistBooks::newNode(bookNodePtr& here)
{
here->next = head;          
head = here;                        
}

However then in you loadFromFile function you will not have to delete the node 'here'

i.e. your function will be:

bool llistBooks::loadFromFile(string fileName)
{
    ifstream in;
    in.open(fileName.c_str());

    if(!in)
    {
         cout << "There was a problem loading files";
         return false;  //this could be useful.
    }
    else
    {
         string Iprice, Icost;
         double the_double;
         bookNodePtr here = new bookNode();   //you missed the parenthases

         while(!in.eof())
         {
             getline(in,here->title,'\n');
             getline(in,here->author, '\n');
             getline(in,here->publisher,'\n');
             getline(in,here->isbn, '\n');
             getline(in,Iprice, '\n');
             the_double = strtod(Iprice.c_str(), NULL);
             here->price = the_double;
             getline(in,Icost, '\n');
             the_double = strtod(Icost.c_str(), NULL);
             here->cost = the_double;
             this->newNode(here);
         }                                                                    
     //here = NULL;
    //delete here;
    }
    in.close();
    return true;
}



Dhyanesh

Dhyanesh: when i do as suggested, it continually loads the last item from file... it is not finding "eof" anymore!

puckerhoop,

Do you *have to* use the newNode function? If not, do you have an insert function? I think I suggested this some time ago, but I'm gonna say this again:

Assume you have an insert function that takes in an instance of the bookNode and insert its information into the list.
something like
void llistBooks::insert(const &bookNode newItem) {
//...
}
then you can do

bool llistBooks::loadFromFile(string fileName)
{
    ifstream in;
    in.open(fileName.c_str());

    if(!in)
    {
         cout << "There was a problem loading files";
         return false;
    }
    else
    {
         string Iprice, Icost;
         bookNodePtr here = new bookNode;

         while(!in.eof())
         {
             getline(in,here->title,'\n');
             getline(in,here->author, '\n');
             getline(in,here->publisher,'\n');
             getline(in,here->isbn, '\n');
             getline(in,Iprice, '\n');          
             here->price = strtod(Iprice.c_str(), NULL);
             getline(in,Icost, '\n');
             here->cost = strtod(Icost.c_str(), NULL);
             insert(&here);
         }
                                             
         here = NULL;
    }
    in.close();
    return true;
}

I have such a function, but that function is not constructed to handle this situation.  It is structured to handle the same type of inputs from display.  
That is why I created the newNode function.
The other option would be to create the Node within this function, but I have not been able to get that to work correctly.

then you can try:
   //assume list is empty.
   else
    {
         string Iprice, Icost;
         head = new bookNode;    //get in the first item so that you can assign to head
         getline(in,head->title,'\n');
         getline(in,head->author, '\n');
         getline(in,head->publisher,'\n');
         getline(in,head->isbn, '\n');
         getline(in,Iprice, '\n');          
         head->price = strtod(Iprice.c_str(), NULL);
         getline(in,Icost, '\n');
         head->cost = strtod(Icost.c_str(), NULL);        

         bookNodePtr here = head->next;

         while(!in.eof())
         {
             here = new bookNode;
             getline(in,here->title,'\n');
             getline(in,here->author, '\n');
             getline(in,here->publisher,'\n');
             getline(in,here->isbn, '\n');
             getline(in,Iprice, '\n');          
             here->price = strtod(Iprice.c_str(), NULL);
             getline(in,Icost, '\n');
             here->cost = strtod(Icost.c_str(), NULL);
             here = here->next;
         }
                                             
         here = NULL;
    }

Let me know if that'll work.

from the way i have structured this function, with the getline feeding in the data... the variables that my insert function requires are considered undeclared within the above function.
I am trying to do this without redoing this entire function.

You probably have understood that load data from file to an empty list is not different than continuously inserting into that list, thus making your loadfromFile function inevitably similar to an insert function (that is, if you do not want to use or do not have an insert function that does that inserting job).

I'm not quite sure what you mean by
>>the variables that my insert function requires are considered undeclared within the above function.

Prior to this function an empty list is created.
When I do as suggested, nothing is loading.

If you don't mind, I would like to see the entire files (header, implementation, driver, and data file). It would be easier to pinpoint the problem if I could look at the code as a whole. Trying to trouble-shoot like this is kinda hard to do.

DRIVER:
#include <string>
#include <iostream>
#include <fstream>
using namespace std;
#include "llistBook.h"            //llistBook class definition

void main ()
{
string temp;
string temp2;                        
string book1 = "book1.txt";            //input filename
string book2 = "book2.txt";            //output filename
string book3 = "book3.txt";            //output filename
string      t_title, t_author, t_isbn, t_publisher;      
double t_price, t_cost;                                                
llistBooks test;            //declaration of an empty list of books

test.loadFromFile(book1);            //load data from file

t_title = "Great Expectations";      //start addition of new book
t_author = "Charles Dickens";
t_isbn = "0-393-96069-2";
t_publisher = "Norton";
t_price = 21.95;
t_cost = 15.01;
test.insertB(t_title, t_author, t_isbn, t_publisher, t_price, t_cost);            //insert new book

test.printAll();                  //print all to screen

temp = "0-13-095990-1";
test.deleteB(temp);                  //delete 5th book
      
test.printAll();                  //print all to screen
}

could you please post the other files (header, implementation) class llistBook as well?

HEADER:
#include <string>
#include <iostream>
using namespace std;

#ifndef LLISTBOOKS_H
#define LLISTBOOKS_H

struct bookNode {
string title;                  //title of book
string author;                  //author of book
string publisher;            //publisher of book
string isbn;                  //isbn of book
double price;                  //price of book
double cost;                  //cost of book
bookNode  *next;            //link to next node
};

typedef  bookNode* bookNodePtr;            //typedef declared for bookNode

class llistBooks {
public:
llistBooks();                  //constructor
~llistBooks();                  //destructor
bool insertB(string title, string author, string isbn, string publisher, double price, double cost);
bool deleteB(string &isbn);
bool findB(string &isbn) const;
bool printB(string isbn) const;
bool printAll() const;
bool saveToFile (string fileName) const;
bool loadFromFile(string fileName);
bool empty() const;
void llistBooks::newNode(bookNodePtr& here);

private:
bookNodePtr      head;                  };
{;
#endif

Sorry, not trying to be a pain here =) but how about the code for insertB and printAll?

FUNCTIONS:
#include <stdlib.h>
#include <string>
#include <iostream>
#include <fstream>
using namespace std;
#include "llistBook.h"


llistBooks::llistBooks()            //constructor
{
head = NULL;                        
}
      
llistBooks::~llistBooks()      //destructor
{
if (empty() )                              
cout << "List is already empty." << endl;
  else
  {
   string next;                              
     while (! empty())                                 
     {
     next = deleteB(next);            //delete bookNodes
     }            
   }                              
cout << "You have destroyed all of the books!" << endl;
}


bool llistBooks::insertB(string title, string author, string isbn, string publisher, double price, double cost)
{
   bookNodePtr temp;                              
   temp = head;                        
   temp = new bookNode;                        
   temp->title = title;
   temp->author = author;
   temp->isbn = isbn;
   temp->publisher = publisher;
   temp->price = price;
   temp->cost = cost;
   temp->next = head;                          
   head = temp;                                          
cout << "\n#-#-#-#-#-#-#-#-#-#-#-#-#" << endl;
cout << "BOOK ADDED: " << temp->isbn << endl;
cout << "#-#-#-#-#-#-#-#-#-#-#-#-#" << endl;
return true;
}


bool llistBooks::deleteB(string &target)            //delete
{
   bookNodePtr before = head;                        
   bookNodePtr temp = before->next;                  
   for (before; before != NULL; before = before->next)      //for loop
   for (temp; temp != NULL; temp = temp->next)
   {
      if (temp->isbn == target)                          
      {                                          cout << "\n#############" << endl;            
cout << temp->isbn << "temp" << endl;
cout << before->isbn << "before" << endl;
     temp = before;
     before = temp->next;
     before->next = temp->next;
     cout << "BOOK DELETED" << endl;
     cout << "#############" << endl;
     delete temp;                        
     return true;                        
     }                                    
     else      
      before = before->next;                  
     }                              
   delete temp;                          
   return false;
}

bool llistBooks::loadFromFile(string fileName)
{
ifstream in;                              
in.open(fileName.c_str());                        
if(!in)                                    
   {
   cout << "There was a problem loading files";
   return false;
   }
else
   {
   string Iprice, Icost;                        
   double the_double;                        
   bookNodePtr head;
   head = new bookNode;      
while(!in.eof())
   {                                          getline(in,head->title,'\n');
getline(in,head->author, '\n');
getline(in,head->publisher,'\n');
getline(in,head->isbn, '\n');
getline(in,Iprice, '\n');
the_double = strtod(Iprice.c_str(), NULL);
head->price = the_double;                  
getline(in,Icost, '\n');                        
the_double = strtod(Icost.c_str(), NULL);            
head->cost = the_double;
   }                              
bookNodePtr here = head->next;
while(!in.eof())
{                        
here = new bookNode;
     getline(in,here->title,'\n');
     getline(in,here->author, '\n');
     getline(in,here->publisher,'\n');
     getline(in,here->isbn, '\n');
     getline(in,Iprice, '\n');
     the_double = strtod(Iprice.c_str(), NULL);         
     here->price = the_double;                   
     getline(in,Icost, '\n');                        
     the_double = strtod(Icost.c_str(), NULL);              
     here->cost = the_double;
     here = here->next;                        
    }                              
      here = NULL;                        
      delete here;                        
      }
      in.close();                                    return true;
      }


bool llistBooks::empty() const
{
   return (head == NULL);
}

      
void llistBooks::newNode(bookNodePtr& here)
{                              
   here->next = head;                              
   head = here;                              
}

PRINTALL:
bool llistBooks::printAll() const
{
if (head == NULL)                              
{
cout<<"\nEmpty list.\n";
return false;
}
else
   {
   cout << "\nPRINT OUT OF ALL BOOKS:" << endl;
   cout << "~~~~~~~~~~~~~~~~~~~~~~~~~~" << endl;
   bookNodePtr here = head;                          
   while(here != NULL)                        
{                              
cout << "\n" << here->title;
cout << "\n" << here->author;
cout << "\n" << here->publisher;
cout << "\n" << here->isbn;
cout << "\n" << here->price;
cout << "\n" << here->cost;
cout << endl;
here = here->next;                              
}                                    
delete here;                              
return true;
 }
}

I see, so you are inserting books at the beginning of the list. Those functions look correct. The problem with loadfromFile is:

bool llistBooks::loadFromFile(string fileName) {
    ifstream in;
    in.open(fileName.c_str());
    if(!in) {
       cout << "There was a problem loading files";
       return false;
    }
    else
    {
       string Iprice, Icost;                    
       double the_double;                    
       //Commented out. bookNodePtr head;
       head = new bookNode;  //Using YOUR head pointer.
       //Commented out: while(!in.eof()) {
       getline(in,head->title,'\n');
       getline(in,head->author, '\n');
       getline(in,head->publisher,'\n');
       getline(in,head->isbn, '\n');
       getline(in,Iprice, '\n');
       the_double = strtod(Iprice.c_str(), NULL);
       head->price = the_double;              
       getline(in,Icost, '\n');                    
       the_double = strtod(Icost.c_str(), NULL);
       head->cost = the_double;
                           
       bookNodePtr here = head->next;
       while(!in.eof()) {                    
           here = new bookNode;
           getline(in,here->title,'\n');
           getline(in,here->author, '\n');
           getline(in,here->publisher,'\n');
           getline(in,here->isbn, '\n');
           getline(in,Iprice, '\n');
           the_double = strtod(Iprice.c_str(), NULL);        
           here->price = the_double;                
           getline(in,Icost, '\n');                    
           the_double = strtod(Icost.c_str(), NULL);            
           here->cost = the_double;
           here = here->next;
      }                        
     here = NULL;                    
     //No need to call delete here: delete here;
     }
     in.close();
     return true;
}

See if that would work now.

not quite... i have to go out for a few hours & will play with it some more, when i return.

I have to go find something to eat as well so that works for both of us =)

When I step thru, the line that is causing the problem is at the beginning of the function.  Your suggested code, that should work, as I have already set up bookNode?!
      head = new bookNode;

actually it kicks it to disassembly but seems to load head (although it never prints).  
there is definetely a problem at the end of the function:
            here = here->next;      
it is not incrementing a new node, but overriding the same one over and over again.  I am going to work on this section

oops there might be an error there. It's been a long time:

bool llistBooks::loadFromFile(string fileName) {
    ifstream in;
    in.open(fileName.c_str());
    if(!in) {
       cout << "There was a problem loading files";
       return false;
    }
    else
    {
       string Iprice, Icost;                    
       double the_double;
             
       head = new bookNode;
       getline(in,head->title,'\n');
       getline(in,head->author, '\n');
       getline(in,head->publisher,'\n');
       getline(in,head->isbn, '\n');
       getline(in,Iprice, '\n');
       the_double = strtod(Iprice.c_str(), NULL);
       head->price = the_double;              
       getline(in,Icost, '\n');                    
       the_double = strtod(Icost.c_str(), NULL);
       head->cost = the_double;
                           
       bookNodePtr here = head;

       while(!in.eof()) {                    
           here->next = new bookNode;
           getline(in,here->title,'\n');
           getline(in,here->author, '\n');
           getline(in,here->publisher,'\n');
           getline(in,here->isbn, '\n');
           getline(in,Iprice, '\n');
           the_double = strtod(Iprice.c_str(), NULL);        
           here->price = the_double;                
           getline(in,Icost, '\n');                    
           the_double = strtod(Icost.c_str(), NULL);            
           here->cost = the_double;
           here = here->next;
      }
     here->next = NULL;                    
     }
     in.close();
     return true;
}

that is better, but it only takes me back to where I started... the first data being pulled in from the file is being overriden by the 2nd set of data.  It looks good when I step thru it!

actually, it is better than my original.. it is moving the original empty node to the end of the list, where it should be!

and it is pushing the file in the opposite direction!
it is now loading the file into the list, backwards...
ie.  the node loaded last is the one that is next to the original empty node at the end of the list.
this doesn't matter for my program, i just found it interesting.
i am going to try and see how this plays into the problem with the missing 1st set of data from the file.

I have played with the code and get one of three results:
1- none add
2- when I get the first node to attach... I lose the rest.
3- I get all but the first to attach
I am going to leave it at this point.
Thank you.