Need help with Singly Linked List C++

Hi there.
I have made two classes. Below is Node Class.

Code:

---

class Node
{
public:
    int id;
    char name[20];
    Node *Next;
};

---

Below is Link Class.

Code:

---

class Link
{
private:
    Node *Head;
    Node *Runner;
    Node *Ptr;
public:
    Link();
    //~Link();
    int Add(int id, char name[20]);
    void Display();
};

---

Below is my Add Function. If condition will run only when there is no node exist. Head is pointing to NULL at this time. If there is one or more than one NODES exist, then else condition will be executed.

Code:

---

int Link::Add(int id, char name[20])
{

    if (Head == NULL)
    {
        Head = new (Node);
        Head->id = id;
        strcpy(Head->name, name);
        Head->Next = Head;
        return 0;
    }
    else
    {
        Runner = Head;
        do
        {
            Runner = Runner->Next;
            Runner = new (Node);
            Runner->id = id;
            strcpy(Runner->name, name);
            Runner->Next = NULL;
            Head = Runner;
            return -1;
        }
        while(Runner != NULL);
       
    }
}

---

Below is my display Code.

Code:

---

void Link::Display()
{
    Runner = Head;

    if (Runner != NULL)
    {           
        cout << "Id  : " <<Runner->id<<endl;
        cout << "Name: " <<Runner->name<<endl;
        Runner = Runner->Next;
    }
}

---

This code takes the input and prints only the most recent input. All i want to do is to make a singly linked list which can make 5 or 10 nodes and prints the data of each NODE.

This is my assignment and i am very confused and worried about it. Please help me out.

Regards

Assad

#1 Are you restricted to using char arrays for the name field? Can you not use a std::string?

#2

Code:

---

if (Head == NULL)
{
    Head = new (Node);
    Head->id = id;
    strcpy(Head->name, name);
    Head->Next = Head;
    return 0;
}

---

This creates a circular list. Are you sure you want to do that? You also do not check that dynamic memory allocation was a success, you simply assume the call to new works.

#3

Code:

---

do
{
    Runner = Runner->Next;
    Runner = new (Node);
    Runner->id = id;
    strcpy(Runner->name, name);
    Runner->Next = NULL;
    Head = Runner;
    return -1;
}
while(Runner != NULL);

---

Why do you have a loop that will only ever execute once due to the return statement. The first line of the loop body has you getting the Next member of the list which, since Runner points to Head and Head->Next points to Head (see comment #2), means that Runner still points to Head. The second line of the loop body throws away what Runner was - so why even have the first line - for a newly allocated Node which you again neglect to check for success. Here is the big one, the line before the return statement destroys the linked list by throwing away what Head used to be for the newly allocated Node. The end result is that this code leaks memory like a sieve and will only ever result in a linked list with a single Node no matter how many times you try and add a new Node.

new throws if it fails anyway, so there is no problem in the first if statement.
If you want to be picky, you should allocate a new node first, since we don't want to modify the list unless the allocation succeeds. Doing that would leave the list in an inconsistent state if the allocation fails.

Do you care what order the items are put into the list?