Why doesn't my implementation of linked list work?

Question

After compiling -when the cmdl pops up- it doesn't terminate and waits just as awaiting input

#include <bits/stdc++.h>

using namespace std;

struct node{
    int data;
    node *next;
};

class LinkedList{
private:
    struct node *head;
    struct node *tail;
public:
    LinkedList(int val1,int val2){
        head->next = tail;
        head->data = val1;
        //tail->next = NULL;
        //tail->data = val2;
    }
    add(int val){
        struct node *n = new node;
        n->next = head->next;
        head->next = n;
        n->data = val;
    }
    display(){
        struct node *ptr = head;
        while(ptr->next!=NULL){
            cout<<ptr->data;
            ptr = ptr->next;
        }
    }
};

int main(){
    LinkedList l(1,3);
    for(int i = 0;i<5;i++) l.add(i);
    l.display();
}

What prevents the code from executing as expected? I tried some built-in functions to test the code however none of them responded to the calls and had effect.

That shouldn't even compile. As for running: what does `head` point to in your constructor? — Mat, Mar 16 '20 at 14:20
you exepct `ptr->next` to be `NULL` at some point, but as you never set it to `NULL`, thats not going to happen — 463035818_is_not_a_number, Mar 16 '20 at 14:21
Your code can never have an empty list since you always put something in head. — stark, Mar 16 '20 at 15:23
`struct node *head;` in `c++` you omit the `struct` so `node *head;` is preferred. — drescherjm, Mar 16 '20 at 16:15
You should read [Why `#include ` is bad](https://stackoverflow.com/q/31816095/9254539). — BessieTheCookie, Mar 16 '20 at 16:56

iverss · Accepted Answer · 2020-03-16T14:52:25.427

2

I get an access violation when running this code.

adding

head = new node;

at the beginning of your constructor fixes that.

I would also explicitly initialize head and tail to null like this

private:
     struct node *head = NULL;
     struct node *tail = NULL;

Otherwise they're filled with garbage values and you'll potentially get an infinite loop in your display code.

edited Mar 16 '20 at 14:52

answered Mar 16 '20 at 14:29

iverss

106
7

1

Also worth asking "Why is my constructor adding a node?" Having to supply a an item to the constructor will make this liked list less useful as you now must know what the first item will be when creating the node, and this is a luxury you often do not have. – user4581301 Mar 16 '20 at 15:17

Reymart Pineda · Answer 2 · 2020-03-16T15:39:11.917

-1

You should initialize both ‘head’ and ‘tail’ as null in the constructor because you have not yet added any element to your linked list and thus both are NULL. Best practices that you add some return type to avoid warnings.

#include <iostream>

using namespace std;

struct node{
    int data;
    node *next;
};

class LinkedList{
private:
    node *head = NULL; //initialize head 
    node *tail = NULL; //initialize tail
public:
    LinkedList(int val1,int val2){
        head->next = tail;
        head->data = val1; //??? I guess you mean tail->data = val1;?
        //tail->next = NULL;
        //tail->data = val2;
    }
//specify what type of return if it has no return add void
    void add(int val){ 
        struct node *n = new node;
        n->next = head->next;
        head->next = n;
        n->data = val;
    }
//also here
    void display(){
        node *ptr = head;
        while(ptr->next!=NULL){
            cout<<ptr->data;
            ptr = ptr->next;
        }
    }
};



int main()
{
  LinkedList l(1, 3);
    for(int i = 0;i<5;i++) l.add(i);
    l.display();

}

edited Mar 16 '20 at 15:39

answered Mar 16 '20 at 14:52

Reymart Pineda

1
1

It's recommended by the community guidelines that clarifying questions be submitted in the comment section rather than the answer section. Best of luck! – iverss Mar 16 '20 at 14:57
1

While this code may solve the question, [including an explanation](//meta.stackexchange.com/q/114762) of how and why this solves the problem would really help to improve the quality of your post, and probably result in more up-votes. Remember that you are answering the question for readers in the future, not just the person asking now. Please [edit] your answer to add explanations and give an indication of what limitations and assumptions apply. – Dharman Mar 16 '20 at 15:14
With the current suggestion `head->next = tail;` would attempt to deference a null pointer. The asker is only marginally better off because the undefined behaviour invoked will almost certainly crash the program. Program will still be wrong. – user4581301 Mar 16 '20 at 18:52

Why doesn't my implementation of linked list work?

2 Answers2