-6

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;
}
Rishabh
  • 25
  • 7
  • 2
    Define "not working"? Compile error? Runtime error? Crash? – John3136 Sep 05 '18 at 13:10
  • 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_an_ai Sep 05 '18 at 13:11
  • 1
    I'm guessing that you get an "unknown identifier error" about `head`. – StoryTeller - Unslander Monica Sep 05 '18 at 13:11
  • 2
    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

2 Answers2

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;
john
  • 85,011
  • 4
  • 57
  • 81
0
  1. Your program leaks all the dynamic memory that it allocates. Always free all resources that you allocate.
  2. Don't ever use malloc in C++. Use new expressions instead.
  3. 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)
eerorika
  • 232,697
  • 12
  • 197
  • 326