How to print a simple linked list (C++)?

Question

The code that I have made is this:

struct node
{
    int value;
    node *prev;
    node *next;
};

void play()
{
    node *head = NULL, *temp = NULL, *run = NULL;

    for (int x = 1; x > 10; x++)
    {
        temp = new node();    //Make a new node
        temp -> value = x;    //Assign value of new node
        temp -> prev = NULL;  //Previous node (node before current node)
        temp -> next = NULL;  //Next node (node after current node)
    }
    if (head == NULL) 
    {
        head = temp; //Head -> Temp
    }
    else
    {
        run = head; //Run -> Head
        while (run -> next != NULL)
        {
            run = run -> next; //Go from node to node
        }
        run -> next = temp; //If next node is null, next node makes a new temp
        temp -> prev = run;
    }
    run = head; //Play from start again
    while (run != NULL)  //Printing
    {
        printf("%d\n", run -> value);
        run = run -> next;
    }
}


int main()
{
  play();
  system ("pause");
  return 0;
}

However, it is not working. There is no output (completely blank). How can I get this linked list to print properly? I want it to output:

1 2 3 4 5 6 7 8 9 10

Other options that I have is to make another separate function for the printing or move the whole thing to int main but I have already tried that and it still does not output anything.

Answer 1

For starters there is a typo in the condition of the first for-loop in the function

for (int x = 1; x > 10; x++)
                ^^^^^^

There must be

for (int x = 1; x <= 10; x++)
                ^^^^^^

Secondly the code that tries to add a new node to the list is outside the for-loop. So only the last allocated node will be added to the list. You have to place the code inside the loop.

Also if here is a double-linked list then it is desirable to have a tail node to which a new node will be appended.

And you should free all allocated memory before exiting the function.

The function can look the following way as it is shown in the demonstrative program.

#include <iostream>
#include <cstdlib>

struct node
{
    int value;
    node *prev;
    node *next;
};

void play()
{
    const int N = 10;
    node *head = nullptr, *tail = nullptr;

    for (int i = 0; i < N; i++)
    {
        node *temp = new node{ i + 1, tail, nullptr };

        if (tail == nullptr)
        {
            head = tail = temp;
        }
        else
        {
            tail = tail->next = temp;
        }
    }

    for (node *current = head; current != nullptr; current = current->next)
    {
        std::cout << current->value << ' ';
    }
    std::cout << std::endl;

    while (head != nullptr)
    {
        node *temp = head;
        head = head->next;
        delete temp;
    }
    tail = head;
}

int main() 
{
    play();
    // system("pause");

    return 0;
}

The program output is

1 2 3 4 5 6 7 8 9 10 

You could make the function more flexible by adding one parameter that specifies the number of nodes in the created list instead of using the magic number 10.

For example

void play( int n )
{
    node *head = nullptr, *tail = nullptr;

    for (int i = 0; i < n; i++)
    {
        node *temp = new node{ i + 1, tail, nullptr };

        if (tail == nullptr)
        {
            head = tail = temp;
        }
        else
        {
            tail = tail->next = temp;
        }
    }

    for (node *current = head; current != nullptr; current = current->next)
    {
        std::cout << current->value << ' ';
    }
    std::cout << std::endl;

    while (head != nullptr)
    {
        node *temp = head;
        head = head->next;
        delete temp;
    }
    tail = head;
}

In this case the function can be called for example like

play( 10 );

or

play( 20 );

and so on.

Answer 2

When you run play(), you create 10 new nodes, but you store them nowhere before creating a new one. Thus, you "lose" all the nodes - except the last one, which is still in temp.

Instead, you should do something like:

for (int x = 1; x < 10; x++)
{
    if (temp == nullptr) {
        temp = new node();
        temp -> value = x;
        temp -> prev = nullptr;
        temp -> next = nullptr;
        head = temp;
    } else {
        temp -> next = new node();
        temp -> next -> value = x;
        temp -> next -> prev = temp;
        temp -> next -> next = nullptr;
        temp = temp -> next
    }
}

Then, you can print your linked list as you already do:

run = head; //Play from start again
while (run != nullptr)  //Printing
{
    printf("%d\n", run -> value);
    run = run -> next;
}

As noticed by @Vlad from Moscow, don't forget to free allocated memory before exiting the function.

Note that I use nullptr instead of NULL. It's a C++11 keyword that replaces NULL. Explications are here.

Answer 3

First your program never enters the for loop. Your loop is equivalent to:

int x=1;
while(x>10) { // always false
  // do stuff
  x++;
}

Therefore, temp is NULL, head is initialize to NULL and nothing happens.

Second, the initialization of your list is not in the loop, so at most only the head would be initialized. Move the closing bracket of the for loop at the end of your function (and adjust indentations etc).

In a second time, and if your compiler allows it, you might consider using more C++ idioms instead of C idioms (if your goal is to learn C++), using nullptr, cout, smart pointers... but it's an other story!