Linked List Class failing:

This is a discussion on Linked List Class failing: within the C++ Programming forums, part of the General Programming Boards category; Code: #ifndef __addressList__ #define __addressList__ #include <iostream> using namespace std; typedef struct node { string Name; string DOB; string address; ...

  1. #1
    Registered User
    Join Date
    Jul 2013
    Location
    Germany
    Posts
    465

    Linked List Class failing:

    Code:
    #ifndef __addressList__
    #define __addressList__
    
    #include <iostream>
    
    using namespace std;
    typedef struct node {
        string Name;
        string DOB;
        string address;
        string aniversary;
        node *next;
    } *nodePtr;
    
    class AddressBook {
    private:
        nodePtr head;
        nodePtr current;
        nodePtr temp;
        
    public:
        AddressBook();
        
        void userPromptStatement();
        void AddNode(nodePtr);
        void deleteName(string);
        void EditNameOrDate();
        void PrintAddressBook();
        void GenBirthdayCards(string);
        void GenAnnCard(string);
        void ExitProgram();
        
    };
    
    #endif
    Code:
    #include <iostream>
    #include <string>
    #include <string.h>
    #include "addressList.h"
    using namespace std;
    AddressBook::AddressBook(){
        
        head=NULL;
        current=NULL;
        temp=NULL;
        
    }
    
    void setDOB(string);
    string getDOB();
    
    void setAniversary(string);
    string getAniversary();
    void AddressBook::AddNode(nodePtr temp)
    {
        if (head != NULL) { //list is already made
            current = head; //makes current pointer point to head of the list
            
            while (current->next != NULL)
            {   //while the next of current is not at end
                current = current->next;     //advance to next node
            }
            
            current->next =new node;
            current->next->address=temp->address;
            current->next->aniversary=temp->aniversary;
            current->next->DOB=temp->DOB;
            current->next->Name=temp->Name;
            current->next->next=nullptr;
        }
        else
        {
            head = new node; //now head is the front of the list
      head->address=temp->address;
      head->aniversary=temp->aniversary;
      head->DOB=temp->aniversary;
      head->Name=temp->Name;
      head->next=nullptr;
        }
        
    }
    
    void AddressBook::deleteName(string Delname)
    {
        //nodePtr delPtr = NULL;
        temp = head;
        current=head;
        
    	while (current != NULL && current->Name != Delname)
    	{
            temp = current;
            current=current->next;
        }
        if (current == NULL) {
            cout<<Delname<<"Was not in the list\n";
        }
        else{
      temp->next=current->next;
            delete current;
            cout<<"Name: "<< Delname <<" was deleted.\n";
        }
        
    }
    
    void AddressBook::PrintAddressBook()
    {
        current = head;
    	if (current==nullptr) cout<<"The list is empty\n";
        while (current != NULL) {
            cout<<"NAME: "<<current->Name<<endl;
            cout<<"ADDRESS: "<<current->address<<endl;
      cout<<"BIRTHDAY: "<<current->DOB<<endl;
            cout<<"ANNIVERSARY: "<<current->aniversary<<endl;
            cout<<"\n";
            current= current->next;
        }
        
        
    }
    void AddressBook::EditNameOrDate()
    {
    	cout<<"1. Edit Name: ";
    	cout<<"2. Edit Birthday: ";
        cout<<"3. Edit Anniversary: ";
    	int input;
       
    	cin>>input;
    	if (input==1)
    	{
      cout<<"Enter the name you want to edit: /n";
            string name;
      getline(cin,name,'\n');
      nodePtr curr=head;
      while(curr->next!= NULL && curr->next->Name != name)
      {
       curr=curr->next;
      }
      cout<<"The current name is: "<<curr->next->Name<<endl;
      cout<<"Enter the new name: "<<name;
      curr->next->Name=name;
      cout<<"The name has been successfully changed to "<<curr->next->Name<<endl;
    	}
    	else if (input==2)
    	{
            cout<<"Enter the Birthday you want to edit: ";
      string date;
      getline(cin,date,'\n');
      nodePtr curr=head;
      while(curr->next!=NULL && curr->next->DOB!=date)
      {
       curr=curr->next;
      }
      cout<<"The current name is: "<<curr->next->DOB<<endl;
      cout<<"Enter the new name: "<<date;
      curr->next->DOB=date;
      cout<<"The name has been successfully changed to "<<curr->next->DOB<<endl;
            
    	}
        else if (input==3)
    	{
            cout<<"Enter the Anniversary date you want to edit: ";
      string AnnDate;
      getline(cin,AnnDate,'\n');
      nodePtr curr=head;
      while(curr->next!=NULL && curr->next->aniversary!=AnnDate)
      {
       curr=curr->next;
      }
      cout<<"The current name is: "<<curr->next->aniversary<<endl;
      cout<<"Enter the new name: "<<AnnDate;
      curr->next->aniversary=AnnDate;
      cout<<"The name has been successfully changed to "<<curr->next->aniversary<<endl;
            
    	}
    }
    void AddressBook::GenBirthdayCards(string birthdayBoy)
    {
        cout<<"Dear "<<birthdayBoy<<"\n\n";
        
        cout<<"Hope your birthday is really wonderful and this coming year is the best yet!\n";
        cout<<endl<<endl;
        cout<<"Love,\n";
        cout<<"Joanne\n";
        
    }
    
    void AddressBook::GenAnnCard(string happyCouple)
    {
        cout<<"Dear "<<happyCouple<<endl;
        cout<<endl<<endl;
        cout<<"May your anniversary be the best yet and we wish you more to come\n";
        cout<<endl<<endl;
        cout<<"Love,\n";
        cout<<"Joanna\n";
        
    }
    void AddressBook::userPromptStatement()
    {
        cout<<"Enter 1 to add a new contact"<<endl;
        cout<<"Enter 2 to delete a contect"<<endl;
        cout<<"Enter 3 edit a contect"<<endl;
        cout<<"Enter 4 to print your address book"<<endl;
        cout<<"Enter 5 to generate birthday cards"<<endl;
        cout<<"Enter 6 to generate anniversary cards"<<endl;
        cout<<"Enter 7 to exit the program"<<endl;
        
    }
    
    void AddressBook::ExitProgram()
    {
        
        EXIT_SUCCESS;
        
    }
    Code:
    #include <iostream>
    #include <string>
    #include "addressList.h"
    
    using namespace std;
    
    
    int main()
    {
        
        AddressBook MyAddressBook;
        int userInput;
        //string NAME, BIRTHDAY, ADDY, NEWANIVERSARY;
    	nodePtr temp=new node;
        MyAddressBook.userPromptStatement();
    	string NAME;
    	while(1)
    	{
      cin>>userInput;
            
      switch (userInput) {
       case 1:
        cout<<"NAME: ";
        cin.ignore();
        getline(cin,temp->Name,'\n');
        cout<<"\nBIRTHDAY: ";
        getline(cin,temp->DOB,'\n');
        cout<<"\nADDRESS: ";
        getline(cin,temp->address,'\n');
        cout<<"\nANIVERSARY DATE: ";
        getline(cin,temp->aniversary,'\n');
        MyAddressBook.AddNode(temp);
        MyAddressBook.userPromptStatement();
        break;
       case 2:
        cout<<"Which contact would you like to delete?\n";
        cin>>NAME;
                    
        MyAddressBook.deleteName(NAME);
        MyAddressBook.userPromptStatement();
        break;
       case 3:
        MyAddressBook.EditNameOrDate();  //need help here
        break;
       case 4:
        MyAddressBook.PrintAddressBook();
        MyAddressBook.userPromptStatement();
        break;
                case 5:
        cout<<"Who would you like to send a Birthday card too?\n";
        cin>>NAME;
        MyAddressBook.GenBirthdayCards(NAME);
        MyAddressBook.userPromptStatement();
        break;
                case 6:
        cout<<"Who would you like to send a anniversary card too?\n";
        cin>>NAME;
        MyAddressBook.GenAnnCard(NAME);
        MyAddressBook.userPromptStatement();
        break;
                    
                case 7:
        MyAddressBook.ExitProgram();
        break;
       default:
        break;
      }
    	}
        return 0;
    }
    The errors are:
    Lets say I enter Number 3 to edit a contact line 99 comes up with an error "Thread 1: EXE_BAD_ACCESS (code=1,address=0x60)". I do not even know what that means.

    Delete Contact also is giving me errors. I am can't seem to see the error. The while loop on 99, 114, 130. I get the following error when I try to delete a name:
    Linked Lists(5607,0x7fff73e1d310) malloc: *** error for object 0x100103b40: pointer being freed was not allocated
    *** set a breakpoint in malloc_error_break to debug

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Code:
    nodePtr curr=head;
      while(curr->next!= NULL && curr->next->Name != name)
    Why not check that curr even exists (ie isn't NULL) before checking curr->next?

  3. #3
    Registered User
    Join Date
    Jul 2013
    Location
    Germany
    Posts
    465
    maybe I am just tired being that it is 23.00 but do you mean this:
    Code:
    if (curr!=NULL) {
                curr= new node;
            }
    

  4. #4
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Well, you said case 3 so I went to EditNameOrDate. This is line 98 of your second file.

  5. #5
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,794
    Code:
    #ifndef __addressList__
    Identifiers with double underscores are reserved for the implementation's (your compiler vendor's) use.




    Your addressList header makes use of std::string objects but does not include the appropriate header. Instead it makes the requirement that the source file(s) include said header - and not just that, but that it be included prior to including the addressList header. Such dependencies on what's going on in other files should be avoided. As mentioned, this header makes use of std::string objects and therefore should require the <string> header. Your header also needlessly includes the <iostream> header though nothing present requires it.




    You should avoid putting a "using namespace std" statement in any header, at least at file scope. Doing so can be somewhat dangerous. Instead, you should explicitly qualify any relevant objects as necessary with std::. Within a source file is OK, as long as it is after any #include statements.




    Const correctness - Any member function of a class that does not alter the object in question should be declared as const. For your case, this would appear to affect the PrintAddressBook, GenBirthdayCards, GenAnnCard, userPromptStatement functions. Looking at things more closely however, I wonder why those are member functions at all (except for the PrintAddressBook function), or at least not static member functions. They do nothing with the data in the class and only perform output based on an input.




    Stylistically, I see - and agree with - a lot of talk about removing I/O from within member functions. As an example, your EditNameOrDate function outputs menus and handles some I/O that could be better implemented elsewhere outside the class and contrasts with your main function which does some I/O and then calls the class' member function to operate on the data entered by the user.




    You make no use of anything that would require the <string.h> header, it should be removed. If you did happen to need it, you should prefer the <cstring> form of the header.




    Prefer initialization lists when initializing your class' data members.




    Code:
    void AddressBook::ExitProgram()
    {
         
        EXIT_SUCCESS;
         
    }
    Ok, I had to look this one up.. this is an inappropriate place IMO to place a statement that exits your program. Also, if you use it, you need to include the <cstdlib> header.




    What you new you must(should) delete. Where are the delete statements in your code?




    Don't remove parameter names like you do in your header file. Elysia hates this one quite a bit apparently.
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  6. #6
    Registered User
    Join Date
    Jul 2013
    Location
    Germany
    Posts
    465
    Thank you very much for your detailed response......

  7. #7
    Registered User
    Join Date
    Jun 2013
    Posts
    56
    Tad bit of nitpicking. You shouldn't use temp as a persistent variable in your class. You want to use variable names that are indicating what the variables job is from start to finish of the class lifetime.

    Generally in a linked list you will have: head, tail, current.
    Head always points to the start of the list.
    Tail always indicates the last node.
    Current indicates what you are currently looking at.

    These are all descriptive of what the variable is doing.

    Temp is a throwaway name. You create it in a small function, use it and throw it away at the end of the function.

  8. #8
    Registered User
    Join Date
    Jul 2013
    Location
    Germany
    Posts
    465
    Ok so I am still having issues with this program. Sorry I have not replied sooner but things have been a little hectic. Ok so, if I use a if state saying if
    Code:
    (!curr) { cout<<"Curr doesn't exist";}
    it executes every time. There I decided to make a new node curr which is not working either. I set curr equal to head a few different ways but I seem not to be getting it.

    @Ewiv I have never seen tail used before. Then again I have only been learning c++ for a few months now. I used temp so I would remember it is temporary and used to connect the list again when I delete something in the list.

  9. #9
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    There's nothing wrong with setting curr equal to head. However, if your list is empty then head is NULL (that's basically what it means to be an empty list) so you need to check that before blindly trying to walk down the list. (If you are searching through the list, for instance, you can stop now because an empty list is not going to contain whatever it is you're searching for.)

  10. #10
    Registered User
    Join Date
    Jun 2013
    Posts
    56
    Quote Originally Posted by jocdrew21 View Post
    @Ewiv I have never seen tail used before. Then again I have only been learning c++ for a few months now. I used temp so I would remember it is temporary and used to connect the list again when I delete something in the list.
    If that's the way you intend to use temp your using it correctly, but you should declare it inside of a function and then wave good bye to it at the end of the function. It's not a permanent part of your class, just a variable you use when adding or deleting a node from the tree so declare it there use it and remove it. I believe that is more efficient use of memory then having it live for the duration of the whole object.

  11. #11
    Registered User
    Join Date
    Jul 2013
    Location
    Germany
    Posts
    465
    Code:
    if (input==1)
    	{
      cout<<"Enter the name you want to edit: /n";
            nodePtr curr = head;
            
            if (curr==NULL) {
                curr = new node;
                head->Name=curr->Name;
                head->next=curr->next;
            }
            string name;
      getline(cin,name,'\n');
      
      while(curr->next!= NULL && curr->next->Name != name)
      {
       curr=curr->next;
      }
            
      cout<<"The current name is: "<<curr->next->Name<<endl;
      cout<<"Enter the new name: "<<name;
      curr->next->Name=name;
      cout<<"The name has been successfully changed to "<<curr->next->Name<<endl;
            
            delete(curr);
            }
    Something along these lines?

    I understand that cure doesn't exist and therefore I thought new would be appropriate to handle that. I figure that out by writing if !curr and cout does not exist.
    Naturally I thought I would assign it to head again and point it to the correct object and later delete it at the end.

  12. #12
    Registered User
    Join Date
    Jun 2013
    Posts
    56
    No, you should check for null, if it's null you know the name will not be found so just cout an error and return from the function.

  13. #13
    Registered User
    Join Date
    Jul 2013
    Location
    Germany
    Posts
    465
    Code:
    if (input==1)
    	{
      cout<<"Enter the name you want to edit: /n";
            nodePtr curr = head;
            
            if (curr==NULL) {
                cout<<"Error, list is empty"<<endl;
            }
            string name;
      getline(cin,name,'\n');
      
      while(curr->next!= NULL && curr->next->Name != name)
      {
       curr=curr->next;
      }
            
      cout<<"The current name is: "<<curr->next->Name<<endl;
      cout<<"Enter the new name: "<<name;
      curr->next->Name=name;
      cout<<"The name has been successfully changed to "<<curr->next->Name<<endl;
            
            }
    Yes I did that before and tired it and yes it was working when the list was empty. However when there are node's complete it jumps straight to the while statement and fails. It completely skips line 12 getline....

  14. #14
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Of course line 10 doesn't get skipped. If you've not been careful with your input though you may still have input in the buffer, which getline will therefore use rather than wait for new data. (Hint: line 18 is an example of you not being careful with your input.)
    Last edited by tabstop; 12-20-2013 at 07:37 AM. Reason: Added hint

  15. #15
    Registered User
    Join Date
    Jul 2013
    Location
    Germany
    Posts
    465
    That was a good point so to ensure it was empty I added name.clear();
    I thought that objects were lost when the program ended. I might have misunderstood what you were talking about.

    If curr is not == null and curr is set to head I should be able to use the while loop correctly, right? However there is not way it can execute correctly if the user cannot input "name" first. Even after I cleared the string name I run into the same issue. I have a feeling once I figure this out I will be quite annoyed with myself.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Splitting in a linked list class
    By FloatingButter in forum C++ Programming
    Replies: 14
    Last Post: 03-17-2013, 08:59 AM
  2. Linked list of a class object?....
    By chadsxe in forum C++ Programming
    Replies: 6
    Last Post: 12-08-2005, 02:15 PM
  3. Basic Linked List class
    By ExCoder01 in forum C++ Programming
    Replies: 3
    Last Post: 09-14-2003, 02:15 AM
  4. traversing a linked list with a node and list class
    By brianptodd in forum C++ Programming
    Replies: 2
    Last Post: 04-24-2003, 11:57 AM
  5. STL Linked List of class within class
    By NixPhoeni in forum C++ Programming
    Replies: 3
    Last Post: 11-30-2001, 09:17 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21