Linked list using Class

Question

I have a code, which seems to be working but I am failing to get the values stored in the linked list between the first and the last node, the pointers in between are skipped? And dereferencing these skipped pointer gives me a segfault, here is the code

#include<iostream>
#include <new>
using namespace std;

class list{ 
    int value;
    list* next;

public:    
    list(int a=0, list* b=0) {value=a;next=b;}    
    //~list() {delete next;}
    void newnode(int a, list* tmp) {
        tmp->next=new list;
        tmp=tmp->next;
        cout<<"Address of next: "<<tmp<<'\n';
        tmp->value=a;
    }

    void printlist (list* regist){
        list* tmp;
        tmp=regist; 
        cout<<tmp->value<<'\n';

        while(tmp->next != 0){
            tmp=tmp->next;
            cout<<tmp->value<<'\n';
            cout<<"Address of next: "<<tmp<<'\n';   
        }
    }
};

int main() {
    int first;    
    cout<<"Enter value for origin: \n";
    cin>>first; 
    list* root=new list(first);
    list* tpo=root;
    cout<<"How many numbers to add? \n";

    int choice;
    cin>>choice;

    int num;
    while(choice) {
        cout<<"Enter value: \n";
        cin>>num;    
        root->newnode(num, tpo);
        choice--;  
    }

    cout<<"Do you want me to show you these values, type 1 for yes and 0 for no: \n";
    cin>>choice;

    if(choice) {
        root->printlist(root);
    }
}

On printing the values why does it skip these pointer(nodes) in between?
Are the in between nodes being pointed to destroyed? If so, commenting the destructor, should do the trick, right?

What am I doing wrong?

For one thing, you're not using consistent indentation. So no one has much chance of reading your code. — Fred Larson
– Fred Larson, Commented Aug 1, 2013 at 12:54
root->newnode(num, tpo); never updates root or tpo so each new value replaces the previous one — Jonathan Potter
– Jonathan Potter, Commented Aug 1, 2013 at 12:56

Iosif Murariu · Accepted Answer · 2013-08-01 14:28:12Z

2

1) You always overwrite the 2nd element in your list when you call for more values. You need to change newnode()'s signature to newnode(int a, list*& tmp).

Later edit: another way would be to have the following signature list* newnode(int a, list* tmp) and at the end of the function you'd return tmp;. Then, in the main loop you'd have tpo = root->newnode(num, tpo);. This way tpo would always point to the next element.

2) Also, for freeing the memory list's destructor shouldn't do anything in particular. I'd say you make a static method in your class that deletes a list. Something like this:

public: static void deleteList(list*& root) { list* tmp = root; while (tmp) { tmp = root->next; delete root; root = NULL; root = tmp; } };

and call it like list::deleteList(root);

edited Aug 1, 2013 at 14:28

answered Aug 1, 2013 at 12:58

Iosif Murariu

2,0841 gold badge21 silver badges28 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Stephen Jacob Over a year ago

About point 1, why pass a pointer as a reference, because I thought the value can be changed either if the variable is passed as a reference or as a pointer

Iosif Murariu Over a year ago

You need to pass the pointer by reference because you need to modify the pointer rather than the object that the pointer is pointing to when you do tmp=tmp->next;. If you don't, tmp will still point to the same pointer that tmp was pointing to before entering the function, and I believe that you want tmp to point to tmp->next. Sheesh, I really need to improve my vocabulary :D (I've been using "point" 4 times in the last phrase)

Stephen Jacob Over a year ago

Thank you very much, did some trials to understand what you said. Would definitely have not figured out if you hadn't mentioned it.

ogni42 · Accepted Answer · 2013-08-01 12:57:30Z

2

you are always submitting root to newnode (as is was assigned to tpo) resulting in a list with two elements and an arbitrary number of leaked memory

answered Aug 1, 2013 at 12:57

ogni42

1,2867 silver badges12 bronze badges

3 Comments

Stephen Jacob Over a year ago

I am sorry but I am relatively new to c++, but can i ask how passing an address would be leading to leaked memory?

Iosif Murariu Over a year ago

You always overwrote the address of tmp->next when you did tmp->next=new list;. This happened because tmp->next would always point to the 2nd element in the list, instead of the 2nd, 3rd etc.

Stephen Jacob Over a year ago

Thanks again, your earlier comment was extremely useful in clarifying how pointers work :), and this made more sense now.

maverickosama92 · Accepted Answer · 2013-08-01 13:13:58Z

1

a comprehensive implementation of linked lists, check below link:

http://www.bitsbyta.com/2011/02/how-to-add-node-at-end-of-linked-list-c.html

answered Aug 1, 2013 at 13:13

maverickosama92

2,7252 gold badges24 silver badges38 bronze badges

Collectives™ on Stack Overflow

Linked list using Class

3 Answers 3

3 Comments

3 Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

3 Comments

Comments

Related