Creating single linked list in c++

Question

I am a beginner in coding and i am creating a linked list(without using class concept), but this is not working, please help.

#include<iostream>
#include<malloc.h>
using namespace std;

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

void add(int n){
    node *head = NULL;
    node *temp = (node*)malloc(sizeof(node));
    temp->data = n;
    temp->next = head;
    head=temp;
}
void traverse(){
    node *temp1 = head;
    while(temp1!=NULL){
        cout<<temp1->data;
        temp1 = temp1->next;
    }
}
int main(){
    add(1);
    add(2);
    add(3);
    traverse();
    return 0;
}

no offense, but this code has too many mistakes. Seems like you could use a [good book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) — 463035818_is_not_a_number, Sep 05 '18 at 13:11
I'm guessing that you get an "unknown identifier error" about `head`. — StoryTeller - Unslander Monica, Sep 05 '18 at 13:11
Don't use `malloc`, use `new`. It's never too late or too early to stop writing C in C++. — molbdnilo, Sep 05 '18 at 13:12
Move `node *head = NULL;` above your add function to fix the compile error. — Retired Ninja, Sep 05 '18 at 13:13
You should not be making these kinds of very fundamental mistakes when you've reached the point in your C++ education where you're implementing linked lists. Get [a good book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list), don't skip over the things you think that you already know. — molbdnilo, Sep 05 '18 at 13:16

john · Answer 1 · 2018-09-05T13:23:59.890

1

This is wrong

void add(int n){
    node *head = NULL;
    node *temp = (node*)malloc(sizeof(node));
    ...

If you are going to make a linked list in this way you need to make head a global variable, otherwise it gets created and destroyed each time you call add.

node *head = NULL;

void add(int n){
    node *temp = (node*)malloc(sizeof(node));
    ...

And there's really no reason not to use new in preference to malloc in a C++ program

    node *temp = new node;

edited Sep 05 '18 at 13:23

answered Sep 05 '18 at 13:18

john

72,145
4
50
71

There are _plenty_ of reasons not to use `new` in a C++ program. If you need dynamic allocation, use something sensible like `std::make_unique`. In 2018 `new` should be rare (though some applications do still exist). – Lightness Races in Orbit Sep 05 '18 at 13:21
I would upvote, if not the *"And there really is no reason not to use `new` in a C++ program*". There are plenty of reasons why **not** to use `new`. Use a smart pointer, if you are going to own something – Fureeish Sep 05 '18 at 13:21
@Fureeish I lol'd – Lightness Races in Orbit Sep 05 '18 at 13:22
@LightnessRacesinOrbit 18 secs lol – Fureeish Sep 05 '18 at 13:22
There is a reason not to use new in C++, actually you shouldn't use it in modern C++ which has smart pointers. – heke Sep 05 '18 at 13:23
@heke slipping into third place there – Lightness Races in Orbit Sep 05 '18 at 13:23
@LightnessRacesinOrbit Well I meant as opposed to malloc, updated. – john Sep 05 '18 at 13:23
@john: Somewhat better :P `malloc` should certainly be avoided. – Lightness Races in Orbit Sep 05 '18 at 13:23
@ Lightness Races in Orbit: Damn, should use the refresh button to make sure that "winning" is still possible :D – heke Sep 05 '18 at 13:30
@john you shouldn't use global variable. You can put it as private static and have the function add and traverse friends – Tomer Wolberg Sep 05 '18 at 17:15

score 0 · Answer 2 · answered Sep 05 '18 at 13:22

Your program leaks all the dynamic memory that it allocates. Always free all resources that you allocate.
Don't ever use malloc in C++. Use new expressions instead.
Don't use bare owing pointers. Use std::unique_ptr instead (although, understanding unique pointers requires understanding of classes).

Finally, the problem is that traverse refers to identifier head which you've not declared. Sure, you have local variable head within the scope of add, but local variables only exist within the (potential) scope where they are declared. Futhermore, add isn't constructing a linked list, since it always uses a separate head for each element.

A procedural linked list implementation should take the head of the list as an argument:

void add(int n, node& head)

void traverse(const node& head)

Creating single linked list in c++

2 Answers2