basic linked list program problem

I want to create a link list that has two record,
and insert another record in between the two.
However my program is not working properly in the sense that the linked list did not really re-link after I commanded it to.

Code:

---

/* This program will attempt to insert a record in between two records */

#include <iostream>

using namespace std;

struct list{

char event[20];
int year;
list *next;
}* root, event1, event2;


void insert(list*, char incident[20], int);

int main()
{
 char event[20];
 int year;

 list event1 = { "World War I", 1914};
 list event2 = { "World War II", 1939};

 root = &event1;
 event1.next = &event2;
 event2.next = NULL;

 cout<<"Please enter an event and a date in between 1914 and 1939."<<endl;
 cin>>event>>year;

 insert(root, event, year);

 return 0;
}

 void insert(list *head, char incident[20], int year)
{
 list event_between;
 event_between.event[20] = incident[20];
 event_between.year = year;

 //The pointer in event1 should point to the reference of event_between and relink the linked list
 event1.next =&event_between;
 event_between.next = &event2;
 event2.next=NULL;

 cout<<"The new list has the following entries: "<<endl;
 while(head!=NULL)
 {
 cout<<head->event<<' '<<head->year;
 cout<<endl;
 head=head->next;
 }
}

---

Could you also please give me some pointers how on to reduce the size of this program?
I'm only beginning C++, thanks for all helps

The whole point of linked lists is to have dynamic data. None of yours are dynamic. Furthermore:

Code:

---

void insert(list *head, char incident[20], int year)
{
 list event_between;

---

That is a local variable that is destroyed when your function returns.


Quzah.

Quote:

---

Originally Posted by Hybodus

Could you also please give me some pointers how on to reduce the size of this program?
I'm only beginning C++, thanks for all helps

---

Well, considering this is C++ and not C, you could simplify your program by using the STL and drop all the C stuff. Additionally, there is a std::list which will make life much easier.

Code:

---

#include <iostream>
/*use the STL*/
#include <string>
#include <list>
#include <iterator>

struct node{
        std::string name;
        int year;
};
void addNode(std::list<node>&,node);

int main(void){

        std::list<node>myList; //create list
        std::list<node>::iterator list_it; //iterator for printing list
        node input; //insertion node

       
        std::cout<<"Enter event name and year: ";
        std::cin>>input.name>>input.year;
        while(!std::cin.eof()){ //Terminate input with EOF
                addNode(myList, input);
                std::cout<<"Enter event name and year: ";
                std::cin>>input.name>>input.year;
        }

        //print output
        list_it = myList.begin();
        while(list_it != myList.end()){
                std::cout<<"Event: "<<list_it->name<<" Year: "<<list_it->year<<std::endl;
                ++list_it;
        }
       
        return(0);
}
void addNode(std::list<node>& myList, node mynode){

        std::list<node>::iterator list_it = myList.begin();
       
        while(list_it!=myList.end()){ //walk list and insert based on year
                if(mynode.year < list_it->year)
                        break;
                ++list_it;
        }

        myList.insert(list_it,mynode);
}

---

I suspect that everyone posting questions about linked lists is a student of some sort, and telling them to use the STL is probably not going to help much. Most instructors won't allow it, and (more importantly) how would they ever learn how it works if they don't code it themselves at least once?

Anyways, to the OP:

Your list structure is correct but here's a few points:

Code:

---

struct list{
    char event[20];
    int year;
    list *next;
};

list *head = NULL; // the head of an empty list

void insert(list*, char incident[20], int);

int main()
{
 // This is how you make a new node in the list:
 head = new list;
 strncpy(list->event, "World War II", 20);
 list->year = 1914;
 list->next = NULL; // remember to mark the end of the list
 return 0;
}

---

Quote:

---

Originally Posted by MacNilly

I suspect that everyone posting questions about linked lists is a student of some sort, and telling them to use the STL is probably not going to help much. Most instructors won't allow it, and (more importantly) how would they ever learn how it works if they don't code it themselves at least once?

---

My response was to his question about making his program shorter, which I showed him how to do, using the STL. If there are specific requirements per his homework then he should post them; I am not responsible for his homework.

Quote:

---

Originally Posted by MacNilly

Anyways, to the OP:

Your list structure is correct but here's a few points:

Code:

---

struct list{
    char event[20];
    int year;
    list *next;
};

list *head = NULL; // the head of an empty list

void insert(list*, char incident[20], int);

int main()
{
 // This is how you make a new node in the list:
 head = new list;
 strncpy(list->event, "World War II", 20);
 list->year = 1914;
 list->next = NULL; // remember to mark the end of the list
 return 0;
}

---

---

1. Using global variables is a bad practice and not required for this problem. The anchor of the linked list should be within main.
2. It is not ok to use char arrays vice std::strings in C++, this is extremely poor practice and should never be suggested.
3. In order to make a new node, he should use a function, but not one that you have suggested since your function is broke.

Honestly, it would have been much easier to just rework my example without the STL list container if that is what you are so concerned with. For the future if you are going to correct someone, ensure you are in fact correct.

I suggest this link: Linked List Basics and other topic on this site. This site help me very very so much about Pointer and Linked List. Shortest & Greatest, two words I give for this document. :)