Creation of LInked List

Question

I want to display the string "Jimmy" but nothing appears. What is my mistake?

#include<iostream>
#include<string>

struct Node 
{
    std::string s;
    Node* next;
};
struct  Node* head = NULL;

void insert(const std::string& name) 
{
    struct Node* newnode = (struct Node*)malloc(sizeof(struct Node));
    newnode->s = name;
    newnode->next = NULL;
    head = newnode;
}

void display() 
{
    struct Node* ptr;
    ptr = head;
    while (ptr != NULL) {
        std::cout << ptr->s << std::endl;
    }
}

int main() 
{
    insert("Jimmy");
    display();
    return 0;
}

No outputs are shown in this code. Please give some recommendation. I am still new to this data structure.

You seem to be a C programmer coming over to C++. The two languages are quite distinct. So much so I'd recommend not relying on your C knowledge too much, and picking up a book from [our curated book list](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). — StoryTeller - Unslander Monica, Jun 09 '19 at 11:32
If you need a linked list you should just use [std::list](https://en.cppreference.com/w/cpp/container/list). But, in most cases, a linked list is a *terribly inefficient* datastructure and you are usually much better off using a [std::vector](https://en.cppreference.com/w/cpp/container/vector). Also; *don't* use `malloc` in C++ and prefer `nullptr` over `NULL`. — Jesper Juhl, Jun 09 '19 at 11:33
Running your code as-is, on my machine leads to an infinite loop that keeps printing `Jimmy`. I assume this is because in `display`, you don't advance `ptr` to `ptr->next`. — Paul92, Jun 09 '19 at 11:34
The first mistake is using `malloc`. You cannot use `malloc` to allocate non-trivial C++ types, of which `std::string` is one. As a rule of thumb, never use `malloc` or `free`. — n. m. could be an AI, Jun 09 '19 at 11:58

score 0 · Answer 1 · answered Jun 09 '19 at 12:23

The standard C function malloc allocates raw memory knowing nothing about the object that will be placed in the memory.

So neither constructor of the object will be called.

The structure Node contains data member of the type std::string for which a constructor shall be called.

In C++ use the operator new instead of calling the C function malloc. The operator not only allocates memory but also calls a constructor for the created object.

It is a bad idea to use global objects in function definitions without passing them through parameters.

The function display can have an infinite loop in case when head is not equal to null pointer because the variable ptr (that is assigned with head) used in the loop is not changed.

void display() 
{
    struct Node* ptr;
    ptr = head;
    while (ptr != NULL) {
        std::cout << ptr->s << std::endl;
    }
}

The function insert can be called only once

void insert(const std::string& name) 
{
    struct Node* newnode = (struct Node*)malloc(sizeof(struct Node));
    newnode->s = name;
    newnode->next = NULL;
    head = newnode;
}

because otherwise it can result in memory leaks

You should free nodes before exiting the program.

Here is a modified your program that does not have the drawbacks of the original program.

#include <iostream>
#include <string>

struct Node
{
    std::string s;
    Node *next;
};

void insert( Node * &head, const std::string &s )
{
    head = new Node { s, head };
}

std::ostream & display( const Node * head, std::ostream &os = std::cout )
{
    for ( const Node *current = head; current != nullptr; current = current->next )
    {
        os << current->s << '\n';
    }

    return os;
}

void clear( Node * &head )
{
    while ( head )
    {
        Node *tmp = head;
        head = head->next;
        delete tmp;
    }
}

int main() 
{
    Node *head = nullptr;

    insert( head, "Jimmy" );

    display( head );

    clear( head );

    return 0;
}

Its output is

Jimmy

Even better than using `new`; allocate automatic objects (on the stack) when you can. Use smart pointers (`std::unique_ptr`/`std::shared_ptr`) when you cannot. Manual memory management is so 1990's and error prone. — Jesper Juhl, Jun 09 '19 at 13:17

Creation of LInked List

1 Answers1