self contained linked list

Question

I created a linked list in this method...

class stack
{
    struct node
    {
        int data;
        node *link;
    }*top;

void insert()
{ ... }

void display()
{ ... }

};

and it works fine... Now i am trying to do the same operations with a self contained linked list but I end up with error. This is my code

class Element
{
public:
    Element(const std::string& str)
    {
        head = NULL;
        head -> data = str;
    }
    void Append(const Element& elem)
    {
        node *newnode;
        newnode=new node;
        newnode->data = elem;
        node *target=head;

        while(target->next != NULL)
            target = target->next;

        target -> next = newnode;
    }

private:
    struct node
    {
        string data;
        node *next;
    }*head;
};

void main()
{   
    Element *root = new Element("Hello");

    root->Append(Element("World"));
}

I want to modify only my Element class, but I am not clear.

I may have done some silly mistake in my program, because I am new to data structures and I am confused by online references.

Answer 1

I think your design is wrong. I might be second guessing what your teacher meant, but I think the self contained list should look like this.

class Element
{
public:
    Element(const std::string& str)
    {
        data = str;
        next = NULL;
    }
    void Append(const Element& elem)
    {
        Element* tail = this;
        while (tail->next)
            tail = tail->next;
        tail->next = new Element(elem.data);
    }

private:
    string data;
    Element *next;
};

In other words Element is the node class. I think this is what you teacher meant by 'self contained linked list class', and 'Element is not a manager class'.

In the main code you where given notice how the start of the list is in the variable root. But you are trying to hold the start of the list in Element::head as well. So with your design the start of the list is being held in two places, which doesn't make a lot of sense. I don't think this is what you teacher intended, but I could be wrong.

Answer 2

In the constructor -

head = NULL;
head -> data = str;

Code has undefined behavior. You should not access members on a NULL pointer. After head pointing to a proper memory location, you should also do -

head -> next = NULL;

in the constructor for the append operation to reliably work. I think Element::Append should receive the std::string parameter given that you are trying to do -

newnode->data = elem;