Tawfik
Tawfik

Reputation: 59

How to correctly implement a linked list class in C++

I'm trying to implement a simple list in C++ but I'm stuck with writing the add function

When adding one element it works fine but then it just keeps adding the element indefinitely

This is weird because I'm making sure to only add to the tail of the linked list and the last element is always NULL.

The result is correct only if we add to a list containing just one element.

#include <iostream>

using namespace std;

class List {

    int value;

    public:
    List(int value);
    List* next;
    List* tail();
    void add(int value);
    void reverse();
    void echo();
};


List::List(int value){
    List::value = value;
    List::next = NULL;
}
List* List::tail(){
    List* tail = this;
    for(;tail->next!=NULL; tail = tail->next);
    return tail;
}
void List::add(int value)
{
    List next (value);
    List* tail = List::tail();
    tail->next = &next;
}
void List::echo()
{
    cout << List::value << "    ";

    if(next!=NULL){
        next->echo();
    }
}

int main(){
    List list (3);
    list.add(1);
    list.add(2);

    list.echo();
}

Upvotes: 0

Views: 106

Answers (2)

ShadowMitia
ShadowMitia

Reputation: 2533

The problem is this part of add : List next (value);. Overall your add function is correct, but you're creating an object that once you exit the function, will be destroyed. When you're assigning tail->next = &next;, inside the add function it's fine, but once you exit the function, that piece of memory doesn't exist anymore.

You have to create a new piece of memory somehow, for example using List* next = new List(value);.

And now that you have manually allocated memory, you need to delete all of them once your object is destroyed, to release the memory, otherwise you'll get a memory leak.

Upvotes: 1

CiaPan
CiaPan

Reputation: 9570

The variable

List next (value);

is automatic, so it exists from the line of its declaration till the end of the add() function. Once the function returns, the variable no longer exists and the pointer you assigned with

tail->next = &next;

becomes invalid.

You should allocate the 'next' item at a heap:

List* next = new List(value);

and append it to the list with

tail->next = next;

Upvotes: 1

Related Questions